* [PATCH 0/2] ksmbd: connection accounting and session teardown fixes
@ 2026-04-18 17:28 DaeMyung Kang
2026-04-18 17:28 ` [PATCH 1/2] ksmbd: fix active_num_conn leak when alloc_transport() fails DaeMyung Kang
2026-04-18 17:28 ` [PATCH 2/2] ksmbd: reset rcount per connection in ksmbd_conn_wait_idle_sess_id() DaeMyung Kang
0 siblings, 2 replies; 5+ messages in thread
From: DaeMyung Kang @ 2026-04-18 17:28 UTC (permalink / raw)
To: linkinjeon, smfrench
Cc: senozhatsky, tom, linux-cifs, linux-kernel, stable,
Henrique Carvalho, DaeMyung Kang
Two independent correctness fixes in the ksmbd server.
1/2 ksmbd_tcp_new_connection() does not decrement active_num_conn on
the alloc_transport() failure path, so repeated allocation
failures monotonically inflate the counter until max_connections
is reached and new clients are refused indefinitely. This is
the remaining half of the same family of accounting bugs
addressed by 77ffbcac4e56 ("smb: server: fix leak of
active_num_conn in ksmbd_tcp_new_connection()"), which only
closed the kthread_run() failure path. Reproduced under a debug
build that forces alloc_transport() to return NULL for a bounded
number of calls; details in the commit log.
2/2 ksmbd_conn_wait_idle_sess_id() stores its per-connection
threshold (rcount) in cross-iteration state, so whether a given
sibling connection is compared against the loose (< 2) or the
strict (< 1) threshold is decided by hash iteration order
relative to curr_conn. Connections visited after curr_conn can
slip through the idle check while still processing requests
against the same session, reopening the teardown race
destroy_previous_session() was meant to close. This is a
code-inspection fix; the iteration-order dependency makes a
targeted reproducer impractical.
The two patches are independent; the series order is not significant.
DaeMyung Kang (2):
ksmbd: fix active_num_conn leak when alloc_transport() fails
ksmbd: reset rcount per connection in ksmbd_conn_wait_idle_sess_id()
fs/smb/server/connection.c | 5 ++---
fs/smb/server/transport_tcp.c | 2 ++
2 files changed, 4 insertions(+), 3 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 1/2] ksmbd: fix active_num_conn leak when alloc_transport() fails 2026-04-18 17:28 [PATCH 0/2] ksmbd: connection accounting and session teardown fixes DaeMyung Kang @ 2026-04-18 17:28 ` DaeMyung Kang 2026-04-19 7:30 ` Namjae Jeon 2026-04-18 17:28 ` [PATCH 2/2] ksmbd: reset rcount per connection in ksmbd_conn_wait_idle_sess_id() DaeMyung Kang 1 sibling, 1 reply; 5+ messages in thread From: DaeMyung Kang @ 2026-04-18 17:28 UTC (permalink / raw) To: linkinjeon, smfrench Cc: senozhatsky, tom, linux-cifs, linux-kernel, stable, Henrique Carvalho, DaeMyung Kang ksmbd_kthread_fn() increments active_num_conn right after accept(), before calling ksmbd_tcp_new_connection(). The decrement normally happens in ksmbd_tcp_disconnect() at the end of the connection's lifetime. If alloc_transport() fails in ksmbd_tcp_new_connection(), the function releases the socket and returns -ENOMEM without going through ksmbd_tcp_disconnect(), so active_num_conn never gets decremented. Under memory pressure, repeated failures monotonically inflate the counter until max_connections is reached and new clients are refused indefinitely. Decrement active_num_conn on this error path, matching the accounting rule used by ksmbd_kthread_fn() and ksmbd_tcp_disconnect(). Commit 77ffbcac4e56 ("smb: server: fix leak of active_num_conn in ksmbd_tcp_new_connection()") fixed the sibling leak on the kthread_run() failure path; this patch closes the remaining one. Reproduced with a debug build that adds a temporary module parameter guarding an early return at the top of alloc_transport(), forcing the first N accept-time transport allocations to fail: * Configure ksmbd with "max connections = 3". * Force 5 successive alloc_transport() failures at the accept path. * Without the fix: active_num_conn drifts up to max_connections and subsequent legitimate mount.cifs attempts are refused with "ksmbd: Limit the maximum number of connections(3)" in dmesg. * With the fix: the counter is correctly decremented on each failure and legitimate mounts continue to succeed. Tested by injecting 5 alloc_transport() failures with max_connections=3 and verifying that subsequent mount.cifs attempts still succeed on the patched kernel while the unpatched kernel refuses them. Fixes: 0d0d4680db22 ("ksmbd: add max connections parameter") Cc: stable@vger.kernel.org Signed-off-by: DaeMyung Kang <charsyam@gmail.com> --- fs/smb/server/transport_tcp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/smb/server/transport_tcp.c b/fs/smb/server/transport_tcp.c index 7e29b06820e2..400412444838 100644 --- a/fs/smb/server/transport_tcp.c +++ b/fs/smb/server/transport_tcp.c @@ -182,6 +182,8 @@ static int ksmbd_tcp_new_connection(struct socket *client_sk) t = alloc_transport(client_sk); if (!t) { + if (server_conf.max_connections) + atomic_dec(&active_num_conn); sock_release(client_sk); return -ENOMEM; } -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] ksmbd: fix active_num_conn leak when alloc_transport() fails 2026-04-18 17:28 ` [PATCH 1/2] ksmbd: fix active_num_conn leak when alloc_transport() fails DaeMyung Kang @ 2026-04-19 7:30 ` Namjae Jeon 0 siblings, 0 replies; 5+ messages in thread From: Namjae Jeon @ 2026-04-19 7:30 UTC (permalink / raw) To: DaeMyung Kang Cc: smfrench, senozhatsky, tom, linux-cifs, linux-kernel, stable, Henrique Carvalho On Sun, Apr 19, 2026 at 2:30 AM DaeMyung Kang <charsyam@gmail.com> wrote: > > ksmbd_kthread_fn() increments active_num_conn right after accept(), > before calling ksmbd_tcp_new_connection(). The decrement normally > happens in ksmbd_tcp_disconnect() at the end of the connection's > lifetime. > > If alloc_transport() fails in ksmbd_tcp_new_connection(), the function > releases the socket and returns -ENOMEM without going through > ksmbd_tcp_disconnect(), so active_num_conn never gets decremented. > Under memory pressure, repeated failures monotonically inflate the > counter until max_connections is reached and new clients are refused > indefinitely. > > Decrement active_num_conn on this error path, matching the accounting > rule used by ksmbd_kthread_fn() and ksmbd_tcp_disconnect(). > > Commit 77ffbcac4e56 ("smb: server: fix leak of active_num_conn in > ksmbd_tcp_new_connection()") fixed the sibling leak on the kthread_run() > failure path; this patch closes the remaining one. > > Reproduced with a debug build that adds a temporary module parameter > guarding an early return at the top of alloc_transport(), forcing > the first N accept-time transport allocations to fail: > > * Configure ksmbd with "max connections = 3". > * Force 5 successive alloc_transport() failures at the accept path. > * Without the fix: active_num_conn drifts up to max_connections and > subsequent legitimate mount.cifs attempts are refused with > "ksmbd: Limit the maximum number of connections(3)" in dmesg. > * With the fix: the counter is correctly decremented on each > failure and legitimate mounts continue to succeed. > > Tested by injecting 5 alloc_transport() failures with > max_connections=3 and verifying that subsequent mount.cifs attempts > still succeed on the patched kernel while the unpatched kernel > refuses them. > > Fixes: 0d0d4680db22 ("ksmbd: add max connections parameter") > Cc: stable@vger.kernel.org > Signed-off-by: DaeMyung Kang <charsyam@gmail.com> Looks good, but Michael Bommarito has already submitted the same patch to the list, and it has been merged into the ksmbd-for-next branch as shown below. https://github.com/smfrench/smb3-kernel/commit/6551300dc452ac16a855a83dbd1e74899542d3b3 Thanks! ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] ksmbd: reset rcount per connection in ksmbd_conn_wait_idle_sess_id() 2026-04-18 17:28 [PATCH 0/2] ksmbd: connection accounting and session teardown fixes DaeMyung Kang 2026-04-18 17:28 ` [PATCH 1/2] ksmbd: fix active_num_conn leak when alloc_transport() fails DaeMyung Kang @ 2026-04-18 17:28 ` DaeMyung Kang 2026-04-19 7:29 ` Namjae Jeon 1 sibling, 1 reply; 5+ messages in thread From: DaeMyung Kang @ 2026-04-18 17:28 UTC (permalink / raw) To: linkinjeon, smfrench Cc: senozhatsky, tom, linux-cifs, linux-kernel, stable, Henrique Carvalho, DaeMyung Kang rcount is intended to be connection-specific: 2 for curr_conn, 1 for every other connection sharing the same session. However, it is initialised only once before the hash iteration and is never reset. After the loop visits curr_conn, later sibling connections are also checked against rcount == 2, so a sibling with req_running == 1 is incorrectly treated as idle. This makes the outcome depend on the hash iteration order: whether a given sibling is checked against the loose (< 2) or the strict (< 1) threshold is decided by whether it happens to be visited before or after curr_conn. The function's contract is "wait until every connection sharing this session is idle" so that destroy_previous_session() can safely tear the session down. The latched rcount violates that contract and reopens the teardown race window the wait logic was meant to close: destroy_previous_session() may proceed before sibling channels have actually quiesced, overlapping session teardown with in-flight work on those connections. Recompute rcount inside the loop so each connection is compared against its own threshold regardless of iteration order. This is a code-inspection fix for an iteration-order-dependent logic error; a targeted reproducer would require SMB3 multichannel with in-flight work on a sibling channel landing after curr_conn in hash order, which is not something that can be triggered reliably. Fixes: 76e98a158b20 ("ksmbd: fix race condition between destroy_previous_session() and smb2 operations()") Cc: stable@vger.kernel.org Signed-off-by: DaeMyung Kang <charsyam@gmail.com> --- fs/smb/server/connection.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/fs/smb/server/connection.c b/fs/smb/server/connection.c index a26899d12df1..b5e077f272cf 100644 --- a/fs/smb/server/connection.c +++ b/fs/smb/server/connection.c @@ -237,7 +237,7 @@ int ksmbd_conn_wait_idle_sess_id(struct ksmbd_conn *curr_conn, u64 sess_id) { struct ksmbd_conn *conn; int rc, retry_count = 0, max_timeout = 120; - int rcount = 1, bkt; + int rcount, bkt; retry_idle: if (retry_count >= max_timeout) @@ -246,8 +246,7 @@ int ksmbd_conn_wait_idle_sess_id(struct ksmbd_conn *curr_conn, u64 sess_id) down_read(&conn_list_lock); hash_for_each(conn_list, bkt, conn, hlist) { if (conn->binding || xa_load(&conn->sessions, sess_id)) { - if (conn == curr_conn) - rcount = 2; + rcount = (conn == curr_conn) ? 2 : 1; if (atomic_read(&conn->req_running) >= rcount) { rc = wait_event_timeout(conn->req_running_q, atomic_read(&conn->req_running) < rcount, -- 2.43.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] ksmbd: reset rcount per connection in ksmbd_conn_wait_idle_sess_id() 2026-04-18 17:28 ` [PATCH 2/2] ksmbd: reset rcount per connection in ksmbd_conn_wait_idle_sess_id() DaeMyung Kang @ 2026-04-19 7:29 ` Namjae Jeon 0 siblings, 0 replies; 5+ messages in thread From: Namjae Jeon @ 2026-04-19 7:29 UTC (permalink / raw) To: DaeMyung Kang Cc: smfrench, senozhatsky, tom, linux-cifs, linux-kernel, stable, Henrique Carvalho On Sun, Apr 19, 2026 at 2:30 AM DaeMyung Kang <charsyam@gmail.com> wrote: > > rcount is intended to be connection-specific: 2 for curr_conn, 1 for > every other connection sharing the same session. However, it is > initialised only once before the hash iteration and is never reset. > After the loop visits curr_conn, later sibling connections are also > checked against rcount == 2, so a sibling with req_running == 1 is > incorrectly treated as idle. This makes the outcome depend on the > hash iteration order: whether a given sibling is checked against the > loose (< 2) or the strict (< 1) threshold is decided by whether it > happens to be visited before or after curr_conn. > > The function's contract is "wait until every connection sharing this > session is idle" so that destroy_previous_session() can safely tear > the session down. The latched rcount violates that contract and > reopens the teardown race window the wait logic was meant to close: > destroy_previous_session() may proceed before sibling channels have > actually quiesced, overlapping session teardown with in-flight work > on those connections. > > Recompute rcount inside the loop so each connection is compared > against its own threshold regardless of iteration order. > > This is a code-inspection fix for an iteration-order-dependent logic > error; a targeted reproducer would require SMB3 multichannel with > in-flight work on a sibling channel landing after curr_conn in hash > order, which is not something that can be triggered reliably. > > Fixes: 76e98a158b20 ("ksmbd: fix race condition between destroy_previous_session() and smb2 operations()") > Cc: stable@vger.kernel.org > Signed-off-by: DaeMyung Kang <charsyam@gmail.com> Applied it to #ksmbd-for-next-next. Thanks! ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-04-19 7:30 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-18 17:28 [PATCH 0/2] ksmbd: connection accounting and session teardown fixes DaeMyung Kang 2026-04-18 17:28 ` [PATCH 1/2] ksmbd: fix active_num_conn leak when alloc_transport() fails DaeMyung Kang 2026-04-19 7:30 ` Namjae Jeon 2026-04-18 17:28 ` [PATCH 2/2] ksmbd: reset rcount per connection in ksmbd_conn_wait_idle_sess_id() DaeMyung Kang 2026-04-19 7:29 ` Namjae Jeon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox