[PATCH] ksmbd: take credits lock for SMB1 negotiate accounting

[PATCH] ksmbd: take credits lock for SMB1 negotiate accounting

[Noah Weinberger]

Hello,

The SMB1 multi-protocol negotiate path increments
conn->outstanding_credits outside conn->credits_lock, while every other
writer of that counter holds the lock. The race is unreachable in
practice (connections in need_neg state are serialized by
queue_ksmbd_work(), and SMB1 messages are rejected before work
allocation once the connection is upgraded), so this is a consistency
fix: move the increment under the existing critical section.

Affected version/commit: mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07), fs/smb/server/smb_common.c:165-183 (ksmbd_verify_smb_message).
Pre-existing in v7.3.

This was found during an assisted (AI-supported) static source review of
mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07); every code path cited below was re-verified by the
reporter against that tree. The patch below has NOT been compiled and has
NOT been runtime tested, and no reproducer was produced - the triggering
conditions are described above instead. Please treat this as a report for
verification rather than a battle-tested fix.

Reported-by: Noah Weinberger <noahweinberger2003@proton.me>
---
diff --git a/fs/smb/server/smb_common.c b/fs/smb/server/smb_common.c
index f9fb374..7453b0f 100644
--- a/fs/smb/server/smb_common.c
+++ b/fs/smb/server/smb_common.c
@@ -166,7 +166,6 @@ int ksmbd_verify_smb_message(struct ksmbd_work *work)
 	    hdr->Command == SMB_COM_NEGOTIATE) {
 		struct ksmbd_conn *conn = work->conn;
 
-		conn->outstanding_credits++;
 		/*
 		 * A legacy SMB1 multi-protocol negotiate occupies sequence
 		 * number 0 but does not pass through
@@ -175,6 +174,7 @@ int ksmbd_verify_smb_message(struct ksmbd_work *work)
 		 * window can advance instead of staying pinned at 0.
 		 */
 		spin_lock(&conn->credits_lock);
+		conn->outstanding_credits++;
 		if (conn->seq_low == 0) {
 			__clear_bit(0, conn->seq_bitmap);
 			conn->seq_low = 1;

Re: [PATCH] ksmbd: take credits lock for SMB1 negotiate accounting

[Ian Bridges]

On Sun, 08 Nov 2026 22:34:09 +0000, Noah Weinberger wrote:
> The SMB1 multi-protocol negotiate path increments
> conn->outstanding_credits outside conn->credits_lock, while every other
> writer of that counter holds the lock.

I can confirm this. smb2_validate_credit_charge(),
smb2_set_rsp_credits() and the release on the send path all update
outstanding_credits under credits_lock. The increment in
ksmbd_verify_smb_message() is the only one outside it. The suggested
change moves it into the critical section that already follows it. The
increment is still returned by smb2_set_rsp_credits() as before.

I also tried to reproduce the issue, with and without the change. After
an SMB1 negotiate, an SMB2 session and a few requests, the counter is
back at 0. Two SMB1 negotiates sent in one write are answered one after
the other. An SMB1 negotiate on a connection that is already on SMB2 is
ignored. This matches your reasoning that the race cannot happen.

Thanks,
Ian