public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.15 01/21] cifs: Correctly set SMB1 SessionKey field in Session Setup Request
@ 2025-06-06 15:41 Sasha Levin
  2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 02/21] cifs: Fix cifs_query_path_info() for Windows NT servers Sasha Levin
                   ` (19 more replies)
  0 siblings, 20 replies; 21+ messages in thread
From: Sasha Levin @ 2025-06-06 15:41 UTC (permalink / raw)
  To: patches, stable
  Cc: Pali Rohár, Steve French, Sasha Levin, sfrench, linux-cifs,
	samba-technical

From: Pali Rohár <pali@kernel.org>

[ Upstream commit 89381c72d52094988e11d23ef24a00066a0fa458 ]

[MS-CIFS] specification in section 2.2.4.53.1 where is described
SMB_COM_SESSION_SETUP_ANDX Request, for SessionKey field says:

    The client MUST set this field to be equal to the SessionKey field in
    the SMB_COM_NEGOTIATE Response for this SMB connection.

Linux SMB client currently set this field to zero. This is working fine
against Windows NT SMB servers thanks to [MS-CIFS] product behavior <94>:

    Windows NT Server ignores the client's SessionKey.

For compatibility with [MS-CIFS], set this SessionKey field in Session
Setup Request to value retrieved from Negotiate response.

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

Based on my comprehensive analysis of both the commit message and code
changes, examining the current kernel tree context: **YES** ## Extensive
Analysis ### **Nature of the Fix** This is a **specification compliance
fix** for SMB1 protocol handling. The commit addresses a clear violation
of the MS-CIFS specification section 2.2.4.53.1, which mandates that the
SessionKey field in SMB_COM_SESSION_SETUP_ANDX Request must be set to
the value from the SMB_COM_NEGOTIATE Response. ### **Code Changes
Analysis** **1. Structural Changes (Low Risk)** - **cifsglob.h**: Adds
`__le32 session_key_id` to `TCP_Server_Info` structure - minimal ABI
impact as this is kernel-internal - **cifspdu.h**: Corrects endianness
annotations from `__u32` to `__le32` - pure annotation fix, no
behavioral change **2. Functional Changes (Contained and Safe)** -
**cifssmb.c**: Captures SessionKey from negotiate response
(`server->session_key_id = pSMBr->SessionKey`) - simple field assignment
- **sess.c**: Sets SessionKey in session setup request
(`pSMB->req.SessionKey = server->session_key_id`) - follows
specification requirement ### **Risk Assessment** **Minimal Regression
Risk:** - Changes are isolated to SMB1 legacy protocol code - Current
behavior (setting SessionKey to zero) works with Windows NT due to MS-
CIFS product behavior <94> - New behavior follows specification exactly,
improving compatibility with non-Windows SMB1 servers - No security
implications - purely a protocol compliance improvement **Code
Maturity:** - SMB1 session setup code is very mature and stable -
Limited recent changes in this area except compatibility fixes - Well-
understood code path with clear specification backing ### **Backport
Suitability Criteria Met** **1. Important Bug Fix:** ✅ Fixes protocol
specification violation that could cause compatibility issues with
strict SMB1 implementations **2. Small and Contained:** ✅ Four minimal
changes across related files, all implementing the same specification
requirement **3. Clear Side Effects:** ✅ No side effects beyond fixing
the specification compliance issue **4. No Architectural Changes:** ✅
Simple field additions and assignments, no structural changes **5. Non-
Critical Subsystem:** ✅ Legacy SMB1 protocol code, deprecated since 2017
**6. Minimal Risk:** ✅ Maintains backward compatibility while improving
forward compatibility **7. Stable Tree Rules Compliance:** ✅ Important
compatibility bugfix with minimal regression risk ### **Comparison to
Similar Commits** This commit closely matches the pattern of **Similar
Commit #3** and **Similar Commit #5** (both marked YES for backporting):
- Protocol compliance improvements - Session setup related fixes - Clear
specification backing - Minimal, contained changes - Authored by
maintainers (Steve French involvement) ### **Conclusion** This commit
represents an ideal stable backport candidate: a clear specification
compliance fix with minimal code changes, no security implications, and
improvement in interoperability. The fix ensures Linux kernel SMB client
properly follows MS-CIFS specification, which is valuable for enterprise
environments using diverse SMB1 server implementations.

 fs/smb/client/cifsglob.h | 1 +
 fs/smb/client/cifspdu.h  | 6 +++---
 fs/smb/client/cifssmb.c  | 1 +
 fs/smb/client/sess.c     | 1 +
 4 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 3b32116b0b496..4d097b074b5fd 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -773,6 +773,7 @@ struct TCP_Server_Info {
 	char workstation_RFC1001_name[RFC1001_NAME_LEN_WITH_NULL];
 	__u32 sequence_number; /* for signing, protected by srv_mutex */
 	__u32 reconnect_instance; /* incremented on each reconnect */
+	__le32 session_key_id; /* retrieved from negotiate response and send in session setup request */
 	struct session_key session_key;
 	unsigned long lstrp; /* when we got last response from this server */
 	struct cifs_secmech secmech; /* crypto sec mech functs, descriptors */
diff --git a/fs/smb/client/cifspdu.h b/fs/smb/client/cifspdu.h
index 1b79fe07476f6..d9cf7db0ac35e 100644
--- a/fs/smb/client/cifspdu.h
+++ b/fs/smb/client/cifspdu.h
@@ -597,7 +597,7 @@ typedef union smb_com_session_setup_andx {
 		__le16 MaxBufferSize;
 		__le16 MaxMpxCount;
 		__le16 VcNumber;
-		__u32 SessionKey;
+		__le32 SessionKey;
 		__le16 SecurityBlobLength;
 		__u32 Reserved;
 		__le32 Capabilities;	/* see below */
@@ -616,7 +616,7 @@ typedef union smb_com_session_setup_andx {
 		__le16 MaxBufferSize;
 		__le16 MaxMpxCount;
 		__le16 VcNumber;
-		__u32 SessionKey;
+		__le32 SessionKey;
 		__le16 CaseInsensitivePasswordLength; /* ASCII password len */
 		__le16 CaseSensitivePasswordLength; /* Unicode password length*/
 		__u32 Reserved;	/* see below */
@@ -654,7 +654,7 @@ typedef union smb_com_session_setup_andx {
 		__le16 MaxBufferSize;
 		__le16 MaxMpxCount;
 		__le16 VcNumber;
-		__u32 SessionKey;
+		__le32 SessionKey;
 		__le16 PasswordLength;
 		__u32 Reserved; /* encrypt key len and offset */
 		__le16 ByteCount;
diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index f55457b4b82e3..f6ec0045c5138 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -498,6 +498,7 @@ CIFSSMBNegotiate(const unsigned int xid,
 	server->max_rw = le32_to_cpu(pSMBr->MaxRawSize);
 	cifs_dbg(NOISY, "Max buf = %d\n", ses->server->maxBuf);
 	server->capabilities = le32_to_cpu(pSMBr->Capabilities);
+	server->session_key_id = pSMBr->SessionKey;
 	server->timeAdj = (int)(__s16)le16_to_cpu(pSMBr->ServerTimeZone);
 	server->timeAdj *= 60;
 
diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
index b3fa9ee269127..d53705595a512 100644
--- a/fs/smb/client/sess.c
+++ b/fs/smb/client/sess.c
@@ -628,6 +628,7 @@ static __u32 cifs_ssetup_hdr(struct cifs_ses *ses,
 					USHRT_MAX));
 	pSMB->req.MaxMpxCount = cpu_to_le16(server->maxReq);
 	pSMB->req.VcNumber = cpu_to_le16(1);
+	pSMB->req.SessionKey = server->session_key_id;
 
 	/* Now no need to set SMBFLG_CASELESS or obsolete CANONICAL PATH */
 
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2025-06-06 15:42 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-06 15:41 [PATCH AUTOSEL 6.15 01/21] cifs: Correctly set SMB1 SessionKey field in Session Setup Request Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 02/21] cifs: Fix cifs_query_path_info() for Windows NT servers Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 03/21] cifs: Fix encoding of SMB1 Session Setup NTLMSSP Request in non-UNICODE mode Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 04/21] NFSv4: Always set NLINK even if the server doesn't support it Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 05/21] NFSv4.2: fix listxattr to return selinux security label Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 06/21] NFSv4.2: fix setattr caching of TIME_[MODIFY|ACCESS]_SET when timestamps are delegated Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 07/21] mailbox: Not protect module_put with spin_lock_irqsave Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 08/21] mfd: max77541: Fix wakeup source leaks on device unbind Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 09/21] mfd: max14577: " Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 10/21] mfd: max77705: " Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 11/21] mfd: 88pm886: " Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 12/21] mfd: sprd-sc27xx: " Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 13/21] sunrpc: don't immediately retransmit on seqno miss Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 14/21] hwmon: (isl28022) Fix current reading calculation Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 15/21] dm vdo indexer: don't read request structure after enqueuing Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 16/21] leds: multicolor: Fix intensity setting while SW blinking Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 17/21] fuse: fix race between concurrent setattrs from multiple nodes Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 18/21] cxl/region: Add a dev_err() on missing target list entries Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 19/21] cxl: core/region - ignore interleave granularity when ways=1 Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 20/21] NFSv4: xattr handlers should check for absent nfs filehandles Sasha Levin
2025-06-06 15:41 ` [PATCH AUTOSEL 6.15 21/21] hwmon: (pmbus/max34440) Fix support for max34451 Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox