From: Dairui Zhang <zhangdairui@gmail.com>
To: Namjae Jeon <linkinjeon@kernel.org>
Cc: linux-cifs@vger.kernel.org, smfrench@gmail.com,
senozhatsky@chromium.org, tom@talpey.com, pc@manguebit.org,
stable@vger.kernel.org, Dairui Zhang <zhangdairui@gmail.com>
Subject: [PATCH v5] ksmbd: verify transform SessionId matches the decrypted header
Date: Wed, 30 Sep 2026 00:19:30 +0800 [thread overview]
Message-ID: <20260929161930.1894929-1-zhangdairui@gmail.com> (raw)
In-Reply-To: <20260928162015.1832109-1-zhangdairui@gmail.com>
Namjae,
You're right. I missed the decompression step in
__handle_ksmbd_work(), and the "no decompression step" and
"no ProtocolId check" claims in the v4 commit message were wrong.
Sorry for the sloppy reading and the extra work it caused you.
v5 moves the check to __handle_ksmbd_work(), after the existing
decompression step, so encrypted compression transforms are no
longer rejected. The transform SessionId is read before
decryption and verified against the final SMB2 message.
The compound-walk bounds check now uses subtraction (the
addition could wrap on 32-bit), and I added the Fixes tag.
From a320c66f1aeb773cf65d2489fae9f982eaf3873f Mon Sep 17 00:00:00 2001
From: Dairui Zhang <zhangdairui@gmail.com>
Date: Tue, 29 Sep 2026 22:30:00 +0800
Subject: [PATCH v5] ksmbd: verify transform SessionId matches the decrypted
header
When an encrypted request comes in, the decryption key is picked by
the SessionId in the transform header, but the request is then
authorized under whatever session the decrypted inner header names.
Nothing ever compares the two, so on a connection with more than
one session a client can get a request decrypted with one session's
key and run it as another session. Encrypted requests are exempt
from signing too, so a valid AEAD tag is the only proof of session
identity, and it is checked against the wrong session.
MS-SMB2 says the server must check that the transform SessionId
matches the one in the decrypted SMB2 header and treat a mismatch
as a protocol error. Add that check in __handle_ksmbd_work() after
decryption, before dispatching: the message has to be at least one
fixed SMB2 header, the first operation must not set
SMB2_FLAGS_RELATED_OPERATIONS and its SessionId must match the
transform SessionId, and every following non-related operation in
a compound chain must carry the same SessionId. SessionId ~0 is
accepted in subsequent operations, same as the existing compound
session check. NextCommand offsets must be 8-byte aligned and stay
inside the message; the bounds check is done by subtraction so it
cannot wrap.
When the decrypted payload is a compression transform, the
existing decompression step runs first and the decompressed SMB2
message is checked instead, since a compression transform header
carries no SessionId.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Reported-by: Dairui Zhang <zhangdairui@gmail.com>
Assisted-by: LLM
Cc: stable@vger.kernel.org
Signed-off-by: Dairui Zhang <zhangdairui@gmail.com>
---
v4 -> v5:
- Move the check into __handle_ksmbd_work(), after the existing
decompression step, instead of rejecting encrypted compression
transforms in smb3_decrypt_req(), per Namjae's review. The
transform SessionId is saved before decryption.
- Use subtraction for the compound-walk bounds check; the
addition form could wrap on 32-bit.
- Add the Fixes tag and drop the incorrect claims about the
decompression step and ProtocolId check from the commit message.
---
fs/smb/server/server.c | 16 ++++++++++++
fs/smb/server/smb2pdu.c | 54 ++++++++++++++++++++++++++++++++++++++++
fs/smb/server/smb2pdu.h | 1 +
3 files changed, 71 insertions(+)
diff --git a/fs/smb/server/server.c b/fs/smb/server/server.c
--- a/fs/smb/server/server.c
+++ b/fs/smb/server/server.c
@@ -187,6 +187,16 @@
if (conn->ops->is_transform_hdr &&
conn->ops->is_transform_hdr(work->request_buf)) {
+ u64 tr_sess_id;
+
+ if (get_rfc1002_len(work->request_buf) <
+ sizeof(struct smb2_transform_hdr)) {
+ ksmbd_conn_abort(conn);
+ return;
+ }
+ tr_sess_id = le64_to_cpu(((struct smb2_transform_hdr *)
+ smb_get_msg(work->request_buf))->SessionId);
+
rc = conn->ops->decrypt_req(work);
if (rc < 0) {
ksmbd_conn_abort(conn);
@@ -215,6 +225,12 @@
ksmbd_conn_abort(conn);
return;
}
+
+ rc = ksmbd_check_transform_session(work, tr_sess_id);
+ if (rc < 0) {
+ ksmbd_conn_abort(conn);
+ return;
+ }
}
if (conn->ops->allocate_rsp_buf(work))
diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -12256,6 +12256,60 @@
return trhdr->ProtocolId == SMB2_TRANSFORM_PROTO_NUM;
}
+/*
+ * The decryption key is selected by the transform SessionId, but
+ * the request is authorized under the session named in the
+ * decrypted SMB2 header. Per MS-SMB2 the two must match, so check
+ * the decrypted message and its compound chain before dispatch.
+ */
+int ksmbd_check_transform_session(struct ksmbd_work *work, u64 tr_sess_id)
+{
+ struct smb2_hdr *hdr = smb_get_msg(work->request_buf);
+ size_t msg_len = get_rfc1002_len(work->request_buf);
+ size_t off = 0;
+ bool first = true;
+
+ if (msg_len < sizeof(*hdr)) {
+ pr_err_ratelimited("Decrypted message is smaller than SMB2 header\n");
+ return -ECONNABORTED;
+ }
+
+ for (;;) {
+ u64 sid = le64_to_cpu(hdr->SessionId);
+ u32 next;
+
+ if (first) {
+ if (hdr->Flags & SMB2_FLAGS_RELATED_OPERATIONS) {
+ pr_err_ratelimited("RELATED_OPERATIONS set on first operation\n");
+ return -ECONNABORTED;
+ }
+ if (sid != tr_sess_id) {
+ pr_err_ratelimited("SessionId mismatch between transform and inner header\n");
+ return -ECONNABORTED;
+ }
+ first = false;
+ } else if (!(hdr->Flags & SMB2_FLAGS_RELATED_OPERATIONS) &&
+ sid != ULLONG_MAX && sid != tr_sess_id) {
+ pr_err_ratelimited("SessionId mismatch in compound chain\n");
+ return -ECONNABORTED;
+ }
+
+ next = le32_to_cpu(hdr->NextCommand);
+ if (!next)
+ return 0;
+ if (next % 8) {
+ pr_err_ratelimited("NextCommand %u is not 8-byte aligned\n", next);
+ return -ECONNABORTED;
+ }
+ if (next > msg_len - off - sizeof(*hdr)) {
+ pr_err_ratelimited("NextCommand %u is out of the message\n", next);
+ return -ECONNABORTED;
+ }
+ off += next;
+ hdr = (struct smb2_hdr *)((u8 *)smb_get_msg(work->request_buf) + off);
+ }
+}
+
int smb3_decrypt_req(struct ksmbd_work *work)
{
char *buf = work->request_buf;
diff --git a/fs/smb/server/smb2pdu.h b/fs/smb/server/smb2pdu.h
--- a/fs/smb/server/smb2pdu.h
+++ b/fs/smb/server/smb2pdu.h
@@ -416,6 +416,7 @@
void smb3_preauth_hash_rsp(struct ksmbd_work *work);
bool smb3_is_transform_hdr(void *buf);
int smb3_decrypt_req(struct ksmbd_work *work);
+int ksmbd_check_transform_session(struct ksmbd_work *work, u64 tr_sess_id);
int smb3_encrypt_resp(struct ksmbd_work *work);
bool smb3_11_final_sess_setup_resp(struct ksmbd_work *work);
int smb2_set_rsp_credits(struct ksmbd_work *work);
--
2.53.0
next parent reply other threads:[~2026-09-29 16:19 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20260928162015.1832109-1-zhangdairui@gmail.com>
2026-09-29 16:19 ` Dairui Zhang [this message]
2026-09-29 17:49 ` [PATCH v6] ksmbd: verify transform SessionId matches the decrypted header Dairui Zhang
2026-10-02 10:36 ` Namjae Jeon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260929161930.1894929-1-zhangdairui@gmail.com \
--to=zhangdairui@gmail.com \
--cc=linkinjeon@kernel.org \
--cc=linux-cifs@vger.kernel.org \
--cc=pc@manguebit.org \
--cc=senozhatsky@chromium.org \
--cc=smfrench@gmail.com \
--cc=stable@vger.kernel.org \
--cc=tom@talpey.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox