From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: "Pali Rohár" <pali@kernel.org>,
"Steve French" <stfrench@microsoft.com>,
"Sasha Levin" <sashal@kernel.org>,
sfrench@samba.org, linux-cifs@vger.kernel.org,
samba-technical@lists.samba.org
Subject: [PATCH AUTOSEL 6.15 03/21] cifs: Fix encoding of SMB1 Session Setup NTLMSSP Request in non-UNICODE mode
Date: Fri, 6 Jun 2025 11:41:28 -0400 [thread overview]
Message-ID: <20250606154147.546388-3-sashal@kernel.org> (raw)
In-Reply-To: <20250606154147.546388-1-sashal@kernel.org>
From: Pali Rohár <pali@kernel.org>
[ Upstream commit 6510ef4230b68c960309e0c1d6eb3e32eb785142 ]
SMB1 Session Setup NTLMSSP Request in non-UNICODE mode is similar to
UNICODE mode, just strings are encoded in ASCII and not in UTF-16.
With this change it is possible to setup SMB1 session with NTLM
authentication in non-UNICODE mode with Windows SMB server.
This change fixes mounting SMB1 servers with -o nounicode mount option
together with -o sec=ntlmssp mount option (which is the default sec=).
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 analysis of both the commit and the kernel source code, I
can now provide a comprehensive assessment: **YES** This commit should
be backported to stable kernel trees. Here's my extensive analysis: ##
Bug Fix Analysis This commit fixes a **functional bug** that prevents
SMB1 sessions from being established when using specific mount options.
The code changes address a critical logic error in the NTLMSSP session
setup for non-UNICODE mode. ### Problem Being Fixed The original code in
`_sess_auth_rawntlmssp_assemble_req()` contains this problematic check:
```c if ((pSMB->req.hdr.Flags2 & SMBFLG2_UNICODE) == 0) { cifs_dbg(VFS,
"NTLMSSP requires Unicode support\n"); return -ENOSYS; } ``` This check
**incorrectly rejects** all non-UNICODE sessions for NTLMSSP
authentication, making it impossible to mount SMB1 servers with: - `-o
nounicode` (non-UNICODE mode) - `-o sec=ntlmssp` (default security mode)
### Technical Solution The fix removes this incorrect validation and
implements proper dual-path handling: ```c if (pSMB->req.hdr.Flags2 &
SMBFLG2_UNICODE) { /bin /bin.usr-is-merged /boot /dev /etc /home /init
/lib /lib.usr-is-merged /lib64 /lost+found /media /mnt /opt /proc /root
/run /sbin /sbin.usr-is-merged /snap /srv /sys /tmp /usr /var unicode
strings must be word aligned */ if
(!IS_ALIGNED(sess_data->iov[0].iov_len + sess_data->iov[1].iov_len, 2))
{ *bcc_ptr = 0; bcc_ptr++; } unicode_oslm_strings(&bcc_ptr,
sess_data->nls_cp); } else { ascii_oslm_strings(&bcc_ptr,
sess_data->nls_cp); } ``` This correctly handles both UNICODE and non-
UNICODE modes by: 1. **UNICODE path**: Uses UTF-16 encoding with proper
alignment 2. **Non-UNICODE path**: Uses ASCII encoding without alignment
requirements ### Why This Qualifies for Backporting 1. **User-Affecting
Bug**: The bug prevents legitimate SMB1 connections with common mount
options, causing mount failures that users would expect to work. 2.
**Small, Contained Fix**: The change is minimal (removes 4 lines, adds 8
lines) and confined to a single function. Risk of regression is very
low. 3. **No Architectural Changes**: This fix doesn't modify the
NTLMSSP protocol implementation or introduce new features—it simply
corrects existing logic. 4. **Similar Pattern to Accepted Backports**:
This closely matches "Similar Commit #1" which was marked "YES" for
backporting. Both commits: - Fix SMB1 Session Setup encoding issues -
Handle UNICODE vs non-UNICODE mode correctly - Extract common code
patterns for consistency - Are small, targeted bugfixes 5. **Legacy
Protocol Support**: While SMB1 is deprecated, it's still supported in
the kernel under `CONFIG_CIFS_ALLOW_INSECURE_LEGACY`, and users who
explicitly enable it deserve working functionality. ### Code Correctness
Verification The fix is technically sound because: -
**`ascii_oslm_strings()`** and **`unicode_oslm_strings()`** are
established functions already used elsewhere in the codebase for
Kerberos authentication (`sess_auth_kerberos()`) - The same dual-path
pattern exists in other session setup functions - The NTLMSSP protocol
itself still uses UNICODE internally (for username/domain encoding), but
the session setup strings can be ASCII - The alignment logic is
correctly applied only to UNICODE mode where it's needed ### Risk
Assessment **Very Low Risk**: - Fix is isolated to SMB1 NTLMSSP session
setup - Only affects the non-UNICODE code path that was previously
broken - No changes to the core NTLMSSP authentication logic - Pattern
matches existing working code in the same file This is a clear bugfix
that restores intended functionality without introducing new features or
architectural changes, making it an ideal candidate for stable tree
backporting.
fs/smb/client/sess.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
index d53705595a512..79b4bd45e31a6 100644
--- a/fs/smb/client/sess.c
+++ b/fs/smb/client/sess.c
@@ -1685,22 +1685,22 @@ _sess_auth_rawntlmssp_assemble_req(struct sess_data *sess_data)
pSMB = (SESSION_SETUP_ANDX *)sess_data->iov[0].iov_base;
capabilities = cifs_ssetup_hdr(ses, server, pSMB);
- if ((pSMB->req.hdr.Flags2 & SMBFLG2_UNICODE) == 0) {
- cifs_dbg(VFS, "NTLMSSP requires Unicode support\n");
- return -ENOSYS;
- }
-
pSMB->req.hdr.Flags2 |= SMBFLG2_EXT_SEC;
capabilities |= CAP_EXTENDED_SECURITY;
pSMB->req.Capabilities |= cpu_to_le32(capabilities);
bcc_ptr = sess_data->iov[2].iov_base;
- /* unicode strings must be word aligned */
- if (!IS_ALIGNED(sess_data->iov[0].iov_len + sess_data->iov[1].iov_len, 2)) {
- *bcc_ptr = 0;
- bcc_ptr++;
+
+ if (pSMB->req.hdr.Flags2 & SMBFLG2_UNICODE) {
+ /* unicode strings must be word aligned */
+ if (!IS_ALIGNED(sess_data->iov[0].iov_len + sess_data->iov[1].iov_len, 2)) {
+ *bcc_ptr = 0;
+ bcc_ptr++;
+ }
+ unicode_oslm_strings(&bcc_ptr, sess_data->nls_cp);
+ } else {
+ ascii_oslm_strings(&bcc_ptr, sess_data->nls_cp);
}
- unicode_oslm_strings(&bcc_ptr, sess_data->nls_cp);
sess_data->iov[2].iov_len = (long) bcc_ptr -
(long) sess_data->iov[2].iov_base;
--
2.39.5
next prev parent reply other threads:[~2025-06-06 15:41 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
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 ` Sasha Levin [this message]
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
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=20250606154147.546388-3-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=linux-cifs@vger.kernel.org \
--cc=pali@kernel.org \
--cc=patches@lists.linux.dev \
--cc=samba-technical@lists.samba.org \
--cc=sfrench@samba.org \
--cc=stable@vger.kernel.org \
--cc=stfrench@microsoft.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