ksmbd: fix use-after-free of conn->preauth_info in concurrent SMB2 NEGOTIATE

conn->preauth_info is shared connection state (struct
preauth_integrity_info, kmalloc-96) that is allocated and freed by the
SMB2 NEGOTIATE handler and read by the response send path.

smb2_handle_negotiate() allocates conn->preauth_info, and on a
deassemble_neg_contexts() failure kfrees it and sets it to NULL. Both the
allocation and the free/NULL happen under ksmbd_conn_lock(conn) (the
connection srv_mutex), which is held across the whole handler body.

The response send path smb3_preauth_hash_rsp(), called from the send:
block of __handle_ksmbd_work(), reads conn->preauth_info and dereferences
conn->preauth_info->Preauth_HashValue (via
ksmbd_gen_preauth_integrity_hash()) without taking conn_lock. When a
client drives two SMB2 NEGOTIATE requests on the same connection, one
worker can free conn->preauth_info on the failing-negotiate path while a
concurrent send-path worker is reading it, producing a slab
use-after-free read (KASAN-confirmed).

The send-path read tested conn->preauth_info for NULL but raced with the
free that occurs between the NULL check and the dereference, so the NULL
guard alone does not close the window.

Serialize the NEGOTIATE-branch read in smb3_preauth_hash_rsp() under
ksmbd_conn_lock(conn) and re-check conn->preauth_info inside the lock.
Because the negotiate handler holds conn_lock across its kfree + NULL
assignment, a reader that also takes conn_lock either runs fully before
the allocation or fully after the NULL store, and can never observe the
freed-but-not-yet-NULLed pointer. ksmbd_gen_preauth_integrity_hash()
takes no locks itself (it only computes a SHA-512 over the buffer), so
no lock-ordering inversion is introduced, and conn_lock is a sleepable
mutex which is safe on this send path (it already performs network I/O).

Fixes: aa7253c2393f ("ksmbd: fix memory leak in smb2_handle_negotiate")
Signed-off-by: Gil Portnoy <dddhkts1@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index f1a0bce0c1f762220aa6b632a8c9cd556ebac7d2..2bc275ed450aea89b03f3332c751e3f1e6a67e7c 100644 (file)
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -9368,10 +9368,13 @@ void smb3_preauth_hash_rsp(struct ksmbd_work *work)
 
        WORK_BUFFERS(work, req, rsp);
 
-       if (le16_to_cpu(req->Command) == SMB2_NEGOTIATE_HE &&
-           conn->preauth_info)
-               ksmbd_gen_preauth_integrity_hash(conn, work->response_buf,
-                                                conn->preauth_info->Preauth_HashValue);
+       if (le16_to_cpu(req->Command) == SMB2_NEGOTIATE_HE) {
+               ksmbd_conn_lock(conn);
+               if (conn->preauth_info)
+                       ksmbd_gen_preauth_integrity_hash(conn, work->response_buf,
+                                                        conn->preauth_info->Preauth_HashValue);
+               ksmbd_conn_unlock(conn);
+       }
 
        if (le16_to_cpu(rsp->Command) == SMB2_SESSION_SETUP_HE && sess) {
                __u8 *hash_value;