public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Namjae Jeon <linkinjeon@kernel.org>,
	Philipp Kerling <pkerling@casix.org>,
	Steve French <stfrench@microsoft.com>,
	Sasha Levin <sashal@kernel.org>,
	smfrench@gmail.com, linux-cifs@vger.kernel.org
Subject: [PATCH AUTOSEL 6.12 01/10] ksmbd: allow a filename to contain special characters on SMB3.1.1 posix extension
Date: Sun,  8 Jun 2025 08:54:58 -0400	[thread overview]
Message-ID: <20250608125507.934032-1-sashal@kernel.org> (raw)

From: Namjae Jeon <linkinjeon@kernel.org>

[ Upstream commit dc3e0f17f74558e8a2fce00608855f050de10230 ]

If client send SMB2_CREATE_POSIX_CONTEXT to ksmbd, Allow a filename
to contain special characters.

Reported-by: Philipp Kerling <pkerling@casix.org>
Signed-off-by: Namjae Jeon <linkinjeon@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 message and code changes,
examining the Linux kernel repository context:

**YES**

This commit should be backported to stable kernel trees. Here's my
extensive analysis:

## Nature of the Change

This is a **legitimate bug fix** that addresses a functional issue where
SMB3.1.1 POSIX extension clients cannot create files with characters
that are valid in POSIX filesystems but blocked by ksmbd's Windows-
centric filename validation.

## Code Analysis

### Key Changes:
1. **Early POSIX context detection** (lines 2906-2925): The code now
   checks for SMB2_CREATE_POSIX_CONTEXT early in the function
2. **Conditional validation bypass**: The critical change at lines
   3161-3164 where `ksmbd_validate_filename(name)` is only called when
   `posix_ctxt == false`
3. **Structural improvement**: Moving POSIX context parsing before
   filename validation, which is the logical order

### What Gets Bypassed:
The `ksmbd_validate_filename()` function normally blocks characters like
`"`, `<`, `>`, `|`, `?`, `*`, and control characters. For POSIX
extensions, these restrictions are inappropriate since POSIX filesystems
can safely handle most of these characters.

## Security Assessment

### Low Security Risk:
1. **Limited Scope**: Only affects clients explicitly using SMB3.1.1
   POSIX extensions (`tcon->posix_extensions` must be enabled)
2. **VFS Protection**: Linux VFS layer provides underlying security
   validation
3. **No Path Traversal**: Higher-level path validation still prevents
   `../` sequences
4. **Standards Compliance**: Aligns with Microsoft's SMB3.1.1 POSIX
   extension specification

### Justification for the Change:
- **User Report**: Philipp Kerling reported this as a functional issue
- **Interoperability**: Essential for proper POSIX-aware SMB client
  functionality
- **Standards Adherence**: SMB3.1.1 POSIX extensions are designed to
  allow POSIX filename semantics

## Comparison with Similar Commits

This follows the pattern of other backported ksmbd commits:
- **Similar to Commit #4**: Another filename validation fix that was
  backported (moving leading slash check)
- **Similar scope**: Limited, well-contained change to specific
  subsystem
- **Bug fix nature**: Addresses functional issue rather than adding new
  features

## Stable Tree Criteria Compliance

✅ **Fixes important bug**: Enables legitimate POSIX client functionality
✅ **Small and contained**: Changes only affect POSIX extension code path
✅ **Minimal regression risk**: Protected by existing VFS validation
✅ **Clear side effects**: Well-understood behavior change
✅ **No architectural changes**: Simple conditional logic modification

## Recommendation

This commit represents a **low-risk bug fix** that enables proper
SMB3.1.1 POSIX extension functionality without introducing significant
security vulnerabilities. The change is well-scoped, addresses a
legitimate user issue, and maintains appropriate security boundaries
through existing VFS-level protections.

 fs/smb/server/smb2pdu.c | 53 +++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 26 deletions(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 08d9a7cfba8cd..815ee5a74901e 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -2870,7 +2870,7 @@ int smb2_open(struct ksmbd_work *work)
 	int req_op_level = 0, open_flags = 0, may_flags = 0, file_info = 0;
 	int rc = 0;
 	int contxt_cnt = 0, query_disk_id = 0;
-	int maximal_access_ctxt = 0, posix_ctxt = 0;
+	bool maximal_access_ctxt = false, posix_ctxt = false;
 	int s_type = 0;
 	int next_off = 0;
 	char *name = NULL;
@@ -2897,6 +2897,27 @@ int smb2_open(struct ksmbd_work *work)
 		return create_smb2_pipe(work);
 	}
 
+	if (req->CreateContextsOffset && tcon->posix_extensions) {
+		context = smb2_find_context_vals(req, SMB2_CREATE_TAG_POSIX, 16);
+		if (IS_ERR(context)) {
+			rc = PTR_ERR(context);
+			goto err_out2;
+		} else if (context) {
+			struct create_posix *posix = (struct create_posix *)context;
+
+			if (le16_to_cpu(context->DataOffset) +
+				le32_to_cpu(context->DataLength) <
+			    sizeof(struct create_posix) - 4) {
+				rc = -EINVAL;
+				goto err_out2;
+			}
+			ksmbd_debug(SMB, "get posix context\n");
+
+			posix_mode = le32_to_cpu(posix->Mode);
+			posix_ctxt = true;
+		}
+	}
+
 	if (req->NameLength) {
 		name = smb2_get_name((char *)req + le16_to_cpu(req->NameOffset),
 				     le16_to_cpu(req->NameLength),
@@ -2919,9 +2940,11 @@ int smb2_open(struct ksmbd_work *work)
 				goto err_out2;
 		}
 
-		rc = ksmbd_validate_filename(name);
-		if (rc < 0)
-			goto err_out2;
+		if (posix_ctxt == false) {
+			rc = ksmbd_validate_filename(name);
+			if (rc < 0)
+				goto err_out2;
+		}
 
 		if (ksmbd_share_veto_filename(share, name)) {
 			rc = -ENOENT;
@@ -3079,28 +3102,6 @@ int smb2_open(struct ksmbd_work *work)
 			rc = -EBADF;
 			goto err_out2;
 		}
-
-		if (tcon->posix_extensions) {
-			context = smb2_find_context_vals(req,
-							 SMB2_CREATE_TAG_POSIX, 16);
-			if (IS_ERR(context)) {
-				rc = PTR_ERR(context);
-				goto err_out2;
-			} else if (context) {
-				struct create_posix *posix =
-					(struct create_posix *)context;
-				if (le16_to_cpu(context->DataOffset) +
-				    le32_to_cpu(context->DataLength) <
-				    sizeof(struct create_posix) - 4) {
-					rc = -EINVAL;
-					goto err_out2;
-				}
-				ksmbd_debug(SMB, "get posix context\n");
-
-				posix_mode = le32_to_cpu(posix->Mode);
-				posix_ctxt = 1;
-			}
-		}
 	}
 
 	if (ksmbd_override_fsids(work)) {
-- 
2.39.5


             reply	other threads:[~2025-06-08 12:55 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-08 12:54 Sasha Levin [this message]
2025-06-08 12:54 ` [PATCH AUTOSEL 6.12 02/10] ksmbd: provide zero as a unique ID to the Mac client Sasha Levin
2025-06-08 12:55 ` [PATCH AUTOSEL 6.12 03/10] rust: module: place cleanup_module() in .exit.text section Sasha Levin
2025-06-08 16:57   ` Miguel Ojeda
2025-06-08 12:55 ` [PATCH AUTOSEL 6.12 04/10] rust: arm: fix unknown (to Clang) argument '-mno-fdpic' Sasha Levin
2025-06-08 16:25   ` Miguel Ojeda
2025-06-08 12:55 ` [PATCH AUTOSEL 6.12 05/10] Revert "iommu/amd: Prevent binding other PCI drivers to IOMMU PCI devices" Sasha Levin
2025-06-08 12:55 ` [PATCH AUTOSEL 6.12 06/10] dmaengine: idxd: Check availability of workqueue allocated by idxd wq driver before using Sasha Levin
2025-06-08 12:55 ` [PATCH AUTOSEL 6.12 07/10] dmaengine: xilinx_dma: Set dma_device directions Sasha Levin
2025-06-08 12:55 ` [PATCH AUTOSEL 6.12 08/10] PCI: dwc: Make link training more robust by setting PORT_LOGIC_LINK_WIDTH to one lane Sasha Levin
2025-06-08 12:55 ` [PATCH AUTOSEL 6.12 09/10] PCI: apple: Fix missing OF node reference in apple_pcie_setup_port Sasha Levin
2025-06-08 12:55 ` [PATCH AUTOSEL 6.12 10/10] PCI: imx6: Add workaround for errata ERR051624 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=20250608125507.934032-1-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=pkerling@casix.org \
    --cc=smfrench@gmail.com \
    --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