[PATCH] ksmbd: fix session reference leak on compounded SESSION_SETUP

[Ian Bridges <icb@fastmail.org>]

A compound request can place SESSION_SETUP after another command. That
earlier command leaves a reference to its session in work->sess.
smb2_sess_setup() then overwrites work->sess with the session it looks up
or creates, and the dispatcher later puts work->sess only once. The
inherited reference is never dropped. That leaks the session, together
with its setup connection and socket, and every repeat of the compound
leaks another.

  unreferenced object (size 512):
    __kmalloc_cache_noprof+0x32c/0x3a0
    ksmbd_smb2_session_create+0xf6/0x9c0 [ksmbd]
    smb2_sess_setup+0xda4/0x6280 [ksmbd]
    handle_ksmbd_work+0x88c/0x1cb0 [ksmbd]

Save the inherited session on entry. After the request has been
processed, release it once, but only when smb2_sess_setup() has replaced
work->sess with its own session or cleared it. Do this after
ksmbd_conn_unlock() so the put does not run under srv_mutex. An error path
may instead leave the inherited session in work->sess. Keep it there, so
the dispatcher still puts it and the error response is still signed or
encrypted with it as before.

Fixes: 7aa8804c0b67 ("ksmbd: fix user-after-free from session log off")
Cc: stable@vger.kernel.org
Signed-off-by: Ian Bridges <icb@fastmail.org>
---

Notes:
    The bug needs a compound in which SESSION_SETUP follows another
    command, for example TREE_CONNECT then SESSION_SETUP. When the later
    SESSION_SETUP replaces or clears work->sess, the inherited session
    reference is leaked. The earlier command only needs an established
    session, not a privileged one, so on a share that maps unknown users
    to guest this is reachable without credentials. A guest SESSION_SETUP
    reaches a usable session, and the compounded SESSION_SETUP that leaks
    need not itself authenticate.
    
    A reproducer is available on request.
    
    This fix was developed with LLM assistance (code review and kmemleak
    reproduction).
    
    Tested on ksmbd-for-next 714f27729121 under KASAN and kmemleak. A
    compound [TREE_CONNECT] + [SESSION_SETUP] repeated on fresh connections
    leaks ksmbd_session objects (traced to ksmbd_smb2_session_create()
    through smb2_sess_setup()) without the patch and none with it. KASAN
    stays quiet. The four reference-balance scenarios (new-session
    replace, failed lookup, same-session reacquire, and a forced
    ksmbd_session_register() failure) each leave no leaked session with
    the patch applied. The register failure was forced with a temporary
    hook that is not part of this patch.

 fs/smb/server/smb2pdu.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index b306bc6e21ee..a21ee045848c 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -2563,6 +2563,8 @@ int smb2_sess_setup(struct ksmbd_work *work)
 	struct smb2_sess_setup_req *req;
 	struct smb2_sess_setup_rsp *rsp;
 	struct ksmbd_session *sess = NULL;
+	struct ksmbd_session *prev_sess = work->sess;
+	bool installed = false;
 	struct negotiate_message *negblob;
 	unsigned int negblob_len, negblob_off;
 	u64 previous_session_id = 0;
@@ -2679,6 +2681,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
 			int sign_ret;
 
 			work->sess = sess;
+			installed = true;
 			if (sess->dialect >= SMB30_PROT_ID)
 				sign_ret = smb3_check_sign_req(work);
 			else
@@ -2731,6 +2734,7 @@ int smb2_sess_setup(struct ksmbd_work *work)
 		conn->binding = false;
 	}
 	work->sess = sess;
+	installed = true;
 
 	negblob_off = le16_to_cpu(req->SecurityBufferOffset);
 	negblob_len = le16_to_cpu(req->SecurityBufferLength);
@@ -2928,6 +2932,14 @@ int smb2_sess_setup(struct ksmbd_work *work)
 	}
 
 	ksmbd_conn_unlock(conn);
+
+	/*
+	 * Later in a compound, work->sess holds the reference that an
+	 * earlier command took. Drop it once this request has installed
+	 * its own session there or cleared it.
+	 */
+	if (prev_sess && (installed || !work->sess))
+		ksmbd_user_session_put(prev_sess);
 	return rc;
 }
 

base-commit: 714f27729121d59f103ca4dad7b0c2d1c125cd71
-- 
2.47.3