Skip to content

Commit

Permalink
ksmbd: fix broken transfers when exceeding max simultaneous operations
Browse files Browse the repository at this point in the history
Since commit 0a77d947f599 ("ksmbd: check outstanding simultaneous SMB
operations"), ksmbd enforces a maximum number of simultaneous operations
for a connection. The problem is that reaching the limit causes ksmbd to
close the socket, and the client has no indication that it should have
slowed down.

This behaviour can be reproduced by setting "smb2 max credits = 128" (or
lower), and transferring a large file (25GB).

smbclient fails as below:

  $ smbclient //192.168.1.254/testshare -U user%pass
  smb: \> put file.bin
  cli_push returned NT_STATUS_USER_SESSION_DELETED
  putting file file.bin as \file.bin smb2cli_req_compound_submit:
  Insufficient credits. 0 available, 1 needed
  NT_STATUS_INTERNAL_ERROR closing remote file \file.bin
  smb: \> smb2cli_req_compound_submit: Insufficient credits. 0 available,
  1 needed

Windows clients fail with 0x8007003b (with smaller files even).

Fix this by delaying reading from the socket until there's room to
allocate a request. This effectively applies backpressure on the client,
so the transfer completes, albeit at a slower rate.

Fixes: 0a77d947f599 ("ksmbd: check outstanding simultaneous SMB operations")
Signed-off-by: Marios Makassikis <[email protected]>
Signed-off-by: Namjae Jeon <[email protected]>
  • Loading branch information
Marios Makassikis authored and namjaejeon committed Dec 9, 2024
1 parent 89b1b00 commit 90b16fc
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 10 deletions.
13 changes: 11 additions & 2 deletions connection.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ struct ksmbd_conn *ksmbd_conn_alloc(void)
atomic_set(&conn->req_running, 0);
atomic_set(&conn->r_count, 0);
atomic_set(&conn->refcnt, 1);
atomic_set(&conn->mux_smb_requests, 0);
conn->total_credits = 1;
conn->outstanding_credits = 0;

Expand Down Expand Up @@ -151,6 +150,8 @@ void ksmbd_conn_try_dequeue_request(struct ksmbd_work *work)
struct ksmbd_conn *conn = work->conn;

atomic_dec(&conn->req_running);
if (waitqueue_active(&conn->req_running_q))
wake_up(&conn->req_running_q);

if (list_empty(&work->request_entry) &&
list_empty(&work->async_request_entry))
Expand Down Expand Up @@ -361,7 +362,7 @@ int ksmbd_conn_handler_loop(void *p)
{
struct ksmbd_conn *conn = (struct ksmbd_conn *)p;
struct ksmbd_transport *t = conn->transport;
unsigned int pdu_size, max_allowed_pdu_size;
unsigned int pdu_size, max_allowed_pdu_size, max_req;
char hdr_buf[4] = {0,};
int size;

Expand All @@ -371,6 +372,7 @@ int ksmbd_conn_handler_loop(void *p)
if (t->ops->prepare && t->ops->prepare(t))
goto out;

max_req = server_conf.max_inflight_req;
conn->last_active = jiffies;
set_freezable();
while (ksmbd_conn_alive(conn)) {
Expand All @@ -380,6 +382,13 @@ int ksmbd_conn_handler_loop(void *p)
kvfree(conn->request_buf);
conn->request_buf = NULL;

recheck:
if (atomic_read(&conn->req_running) + 1 > max_req) {
wait_event_interruptible(conn->req_running_q,
atomic_read(&conn->req_running) < max_req);
goto recheck;
}

size = t->ops->read(t, hdr_buf, sizeof(hdr_buf), -1);
if (size != sizeof(hdr_buf))
break;
Expand Down
1 change: 0 additions & 1 deletion connection.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@ struct ksmbd_conn {
__le16 signing_algorithm;
bool binding;
atomic_t refcnt;
atomic_t mux_smb_requests;
};

struct ksmbd_conn_ops {
Expand Down
7 changes: 1 addition & 6 deletions server.c
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@ static void handle_ksmbd_work(struct work_struct *wk)

ksmbd_conn_try_dequeue_request(work);
ksmbd_free_work_struct(work);
atomic_dec(&conn->mux_smb_requests);
/*
* Checking waitqueue to dropping pending requests on
* disconnection. waitqueue_active is safe because it
Expand Down Expand Up @@ -300,11 +299,6 @@ static int queue_ksmbd_work(struct ksmbd_conn *conn)
if (err)
return 0;

if (atomic_inc_return(&conn->mux_smb_requests) >= conn->vals->max_credits) {
atomic_dec_return(&conn->mux_smb_requests);
return -ENOSPC;
}

work = ksmbd_alloc_work_struct();
if (!work) {
pr_err("allocation for work failed\n");
Expand Down Expand Up @@ -367,6 +361,7 @@ static int server_conf_init(void)
server_conf.auth_mechs |= KSMBD_AUTH_KRB5 |
KSMBD_AUTH_MSKRB5;
#endif
server_conf.max_inflight_req = SMB2_MAX_CREDITS;
return 0;
}

Expand Down
1 change: 1 addition & 0 deletions server.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ struct ksmbd_server_config {
struct smb_sid domain_sid;
unsigned int auth_mechs;
unsigned int max_connections;
unsigned int max_inflight_req;

char *conf[SERVER_CONF_WORK_GROUP + 1];
struct task_struct *dh_task;
Expand Down
5 changes: 4 additions & 1 deletion transport_ipc.c
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,11 @@ static int ipc_server_config_on_startup(struct ksmbd_startup_request *req)
init_smb2_max_write_size(req->smb2_max_write);
if (req->smb2_max_trans)
init_smb2_max_trans_size(req->smb2_max_trans);
if (req->smb2_max_credits)
if (req->smb2_max_credits) {
init_smb2_max_credits(req->smb2_max_credits);
server_conf.max_inflight_req =
req->smb2_max_credits;
}
if (req->smbd_max_io_size)
init_smbd_max_io_size(req->smbd_max_io_size);

Expand Down

0 comments on commit 90b16fc

Please sign in to comment.