[PATCH 0/2] ksmbd: connection accounting and session teardown fixes

[PATCH 0/2] ksmbd: connection accounting and session teardown fixes

[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

[PATCH 1/2] ksmbd: fix active_num_conn leak when alloc_transport() fails

[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

[PATCH 2/2] ksmbd: reset rcount per connection in ksmbd_conn_wait_idle_sess_id()

[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