[PATCH] ksmbd: check sign flag of the current compound command

[PATCH] ksmbd: check sign flag of the current compound command

[Noah Weinberger]

Hello,

smb2_is_sign_req() decides whether a command must be verified/answered
signed based on the FIRST command's header, even when dispatching
commands 2..N of a compound, so the SIGNED flag of the current command
is ignored and its response goes out unsigned.

When the session mandates signing, __process_request rejects the whole
compound because the first header is unsigned, and when signing is
optional an unsigned request was already permitted - so I could not
construct a bypass; this is a conformance/defense-in-depth fix. Use the
current command's request header, like smb2_check_sign_req() does.

Affected version/commit: mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07), fs/smb/server/smb2pdu.c:12811-12820 (smb2_is_sign_req).
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/smb2pdu.c b/fs/smb/server/smb2pdu.c
index b084cca..d0c6356 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -12812,6 +12812,9 @@ bool smb2_is_sign_req(struct ksmbd_work *work, unsigned int command)
 {
 	struct smb2_hdr *rcv_hdr2 = smb_get_msg(work->request_buf);
 
+	if (work->next_smb2_rcv_hdr_off)
+		rcv_hdr2 = ksmbd_req_buf_next(work);
+
 	if ((rcv_hdr2->Flags & SMB2_FLAGS_SIGNED) &&
 	    command != SMB2_NEGOTIATE_HE)
 		return true;

Re: [PATCH] ksmbd: check sign flag of the current compound command

[Ian Bridges]

On Sun, 08 Nov 2026 22:34:08 +0000, Noah Weinberger wrote:
> smb2_is_sign_req() decides whether a command must be verified/answered
> signed based on the FIRST command's header, even when dispatching
> commands 2..N of a compound, so the SIGNED flag of the current command
> is ignored and its response goes out unsigned.

The report is right for the request side. __process_request() runs once
per compound member, and each member's own SIGNED flag should decide
whether its signature is checked. I found no bypass either way. A member
of a session that requires signing is still refused when it is unsigned,
and a signed member is still verified with its own session's key.

The suggested change does not fix the response side, though. It moves
the problem there.

__handle_ksmbd_work() calls is_sign_req() a second time to decide
whether to sign the response. That call comes after
is_chained_smb2_message(). For every command except the last,
is_chained_smb2_message() calls init_chained_smb2_rsp(), which has
already moved next_smb2_rcv_hdr_off to the next request. The response
that gets signed is the one just built, because smb2_set_sign_rsp() and
smb3_set_sign_rsp() sign ksmbd_resp_buf_curr(). With the change, whether
response N is signed therefore depends on the SIGNED flag of request
N+1.

If request N is signed and request N+1 is not, response N goes out
unsigned. If request N is unsigned and request N+1 is signed, response N
goes out signed, although a response to an unsigned request must not be
signed.

This only matters when the session does not require signing, because a
session that requires it signs every response anyway. It also needs a
compound that mixes signed and unsigned commands.

I tested both sides on ksmbd-for-next with compounds of two
TREE_CONNECTs, on a session that does not require signing.

Thanks,
Ian

Re: [PATCH] ksmbd: check sign flag of the current compound command

[Noah Weinberger]

Hi Ian,

Thanks for checking both paths and testing this. I missed that the response-signing call happens after the request offset advances. Please don't apply this version; the request and response paths need to be handled separately.

Thanks,
Noah