smb: client: fix create context out-of-bounds reads

Hi Linux kernel maintainers,

We found and validated a issue in fs/smb/client/smb2pdu.c. The bug is
reachable by a non-root user via user and net namespace.
We've tested it, and it should not affect any other functionality.
The malicious server can be operated by a non-root user in a user and net
namespace. The guest-side CIFS mount in this validation is performed as root
because CIFS does not set FS_USERNS_MOUNT.

We will provide detailed information about the bug
in this email, along with a PoC to trigger it.

---- details below ----

Bug details:

The reproduced crash is a truncated QFid create context. Generic layout
checks accept DataLength 0, then parse_query_id_ctxt() still reads
DiskFileId through a full response-structure cast and crosses the
cifs_small_rq object.

smb2_parse_contexts() also does not bound each context record by its
Next field before dispatching to a handler. The SMB2 lease parser reads
LeaseFlags after LeaseState, so a short lease context can pass the
generic checks and still be handed to smb2_parse_lease_buf(). Patch 1
bounds each context by Next, validates the QFid payload length, and
extends the SMB2 lease minimum through LeaseFlags. The embedded PoC and
KASAN report show the truncated QFid path; the Next and lease checks
close sibling parser holes in the same function.

parse_posix_ctxt() reads nlink, reparse_tag, and mode before checking
that the POSIX context contains those fields. Current in-tree
smb2_open_file() passes a NULL posix pointer into SMB2_open(), so this
handler is not reached on the ordinary open path. Patch 2 still adds a
handler-level minimum DataLength check and keeps the existing
soft-failure behavior for malformed optional metadata.

Please accept my apologies for the follow-up. I posted v2 on 23 Aug 2026
and have not received a reply, so I am resending the series as v3. There
are no functional code changes versus v2.

311 more lines

smb2_parse_contexts() validates the complete create-context area but does
not bound each context record by its Next field before dispatching to a
handler.  A malformed chain can therefore expose bytes past one context
to the handler.  The QFid handler also used a full response-structure
cast even though it only consumes DiskFileId.

Bound each context by Next and reject malformed chains.  Read the QFid
DiskFileId only when the context data covers that field, and do not
call the lease parser unless the record contains every field it reads,
including LeaseFlags.

Fixes: b8c32dbb0deb ("CIFS: Request SMB2.1 leases")
Fixes: 89a5bfa350fa ("smb3: optimize open to not send query file internal info")
Cc: <redacted>
Reported-by: Vega <redacted>
Assisted-by: LLM
Signed-off-by: Zihan Xi <redacted>
---
changes in v3:
  - Resend after no response on v2; no functional code changes.
  - v2 Link: https://lore.kernel.org/all/cover.1787486936.git.zihanx@nebusec.ai/
changes in v2:
  - Bound each response context by Next and reject malformed chains.
  - Read QFid DiskFileId only when DataLength covers the payload.
  - Extend the SMB2 lease minimum through the LeaseFlags field.
  - Correct Fixes history for the pre-existing Next/lease path and the later QFid path.
  - v1 Link: https://lore.kernel.org/all/eb1bc35611f91bd10a4772400b37fac26f660956.1782579150.git.xizh2024@lzu.edu.cn/
---
 fs/smb/client/smb2pdu.c | 38 ++++++++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 6 deletions(-)

diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 4ce165e40657f..95d862a1241be 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c

@@ -2375,14 +2375,29 @@ create_reconnect_durable_buf(struct cifs_fid *fid)
 	return buf;
 }
 
+static size_t smb2_create_lease_min_cc_len(struct TCP_Server_Info *server)
+{
+	if (server->vals->create_lease_size == sizeof(struct create_lease_v2))
+		return offsetof(struct create_lease_v2, lcontext.Epoch) +
+			sizeof(__le16);
+	return offsetof(struct create_lease, lcontext.LeaseFlags) +
+		sizeof(__le32);
+}
+
 static void
 parse_query_id_ctxt(struct create_context *cc, struct smb2_file_all_info *buf)
 {
-	struct create_disk_id_rsp *pdisk_id = (struct create_disk_id_rsp *)cc;
+	u16 doff = le16_to_cpu(cc->DataOffset);
+	u32 dlen = le32_to_cpu(cc->DataLength);
+	u8 *beg;
+
+	if (dlen < sizeof(__le64))
+		return;
 
-	cifs_dbg(FYI, "parse query id context 0x%llx 0x%llx\n",
-		pdisk_id->DiskFileId, pdisk_id->VolumeId);
-	buf->IndexNumber = pdisk_id->DiskFileId;
+	beg = (u8 *)cc + doff;
+	memcpy(&buf->IndexNumber, beg, sizeof(__le64));
+	cifs_dbg(FYI, "parse query id context 0x%llx\n",
+		 le64_to_cpu(buf->IndexNumber));
 }
 
 static void

@@ -2430,6 +2445,7 @@ int smb2_parse_contexts(struct TCP_Server_Info *server,
 	struct smb2_create_rsp *rsp = rsp_iov->iov_base;
 	struct create_context *cc;
 	size_t rem, off, len;
+	size_t cc_len;
 	size_t doff, dlen;
 	size_t noff, nlen;
 	char *name;

@@ -2452,9 +2468,18 @@ int smb2_parse_contexts(struct TCP_Server_Info *server,
 		buf->IndexNumber = 0;
 
 	while (rem >= sizeof(*cc)) {
+		off = le32_to_cpu(cc->Next);
+		if (off) {
+			if ((off & 0x7) || off > rem || off < sizeof(*cc))
+				return -EINVAL;
+			cc_len = off;
+		} else {
+			cc_len = rem;
+		}
+
 		doff = le16_to_cpu(cc->DataOffset);
 		dlen = le32_to_cpu(cc->DataLength);
-		if (check_add_overflow(doff, dlen, &len) || len > rem)
+		if (check_add_overflow(doff, dlen, &len) || len > cc_len)
 			return -EINVAL;
 
 		noff = le16_to_cpu(cc->NameOffset);

@@ -2466,7 +2491,8 @@ int smb2_parse_contexts(struct TCP_Server_Info *server,
 		switch (nlen) {
 		case 4:
 			if (!strncmp(name, SMB2_CREATE_REQUEST_LEASE, 4)) {
-				*oplock = server->ops->parse_lease_buf(cc, epoch,
+				if (cc_len >= smb2_create_lease_min_cc_len(server))
+					*oplock = server->ops->parse_lease_buf(cc, epoch,
 								       lease_key);
 			} else if (buf &&
 				   !strncmp(name, SMB2_CREATE_QUERY_ON_DISK_ID, 4)) {

-- 
2.43.0

parse_posix_ctxt() reads the fixed nlink, reparse-tag, and mode fields
before checking that the POSIX create context contains them.  A short
context can pass the generic create-context checks and still make these
fixed-width reads run past the declared data.

The current in-tree smb2_open_file() path passes a NULL posix pointer,
so this handler is not reached on ordinary open.  Still require the
POSIX context data to cover the three fixed fields before reading them,
because the helper itself performs those unguarded reads.  Keep the
handler's existing soft-failure behavior for malformed metadata so a
bad optional context does not fail the open.

Fixes: 69dda3059e7a ("cifs: add SMB2_open() arg to return POSIX data")
Cc: <redacted>
Reported-by: Vega <redacted>
Assisted-by: LLM
Signed-off-by: Zihan Xi <redacted>
---
changes in v3:
  - Resend after no response on v2; no functional code changes.
  - v2 Link: https://lore.kernel.org/all/cover.1787486936.git.zihanx@nebusec.ai/
changes in v2:
  - Add a handler-level DataLength check before reading the three fixed POSIX fields.
  - Attribute the fixed-field read to the POSIX create-context introduction.
  - Preserve the existing soft-failure behavior for malformed optional metadata.
  - v1 Link: https://lore.kernel.org/all/eb1bc35611f91bd10a4772400b37fac26f660956.1782579150.git.xizh2024@lzu.edu.cn/
---
 fs/smb/client/smb2pdu.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 95d862a1241be..e3973d331633d 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c

@@ -2405,11 +2405,14 @@ parse_posix_ctxt(struct create_context *cc, struct smb2_file_all_info *info,
 		 struct create_posix_rsp *posix)
 {
 	int sid_len;
+	u32 dlen = le32_to_cpu(cc->DataLength);
 	u8 *beg = (u8 *)cc + le16_to_cpu(cc->DataOffset);
-	u8 *end = beg + le32_to_cpu(cc->DataLength);
+	u8 *end = beg + dlen;
 	u8 *sid;
 
 	memset(posix, 0, sizeof(*posix));
+	if (dlen < 3 * sizeof(__le32))
+		return;
 
 	posix->nlink = get_unaligned_le32(beg);
 	posix->reparse_tag = get_unaligned_le32(beg + 4);

-- 
2.43.0

On Fri, Sep 04, 2026 at 02:05:17PM +0000, Zihan Xi wrote:

smb2_parse_contexts() validates the complete create-context area but does
not bound each context record by its Next field before dispatching to a
handler.  A malformed chain can therefore expose bytes past one context to
the handler.  The QFid handler also used a full response-structure cast even
though it only consumes DiskFileId.

Hello,

I have some thoughts/comments on your series.

First, a possible reason for not getting a response on your v2, and
delayed response on this v3: Steve French passed away in August, so
mail addressed to him isn't reaching a maintaner, and things may
have gotten missed during transitions.  You'll want to send future
versions to Paulo Alcantara [off-list ref], with Cc to
Namjae Jeon [off-list ref]


I've been carrying an overlapping patch (an earlier posting:
https://lore.kernel.org/r/20260826153147.4112943-12-sorenson@redhat.com).
in a bounds-checking series of my own.  Like yours, my patch has
been addressing the memory safety problem in the three handlers
which read at fixed offsets, rather than the generic checks.

But I think yours is probably a better fix, and you've got the
PoC, so if we can perfect yours and get it in, I'll be dropping
mine in favor of this series.


A few points (take with a grain of salt):

1) The lease parser still reads at a fixed offset rather than from
   DataOffset:

cc_len bounds the record, so the read stays in bounds.  But this is
just like the QFid bug you just fixed nearby:  smb2_parse_lease_buf()
and smb3_parse_lease_buf() reach the fields at the canonical offset
of the create_lease layout, not at DataOffset, so a valid but non-
canonical DataOffset could get in-bounds garbage rather than an
OOB.  That's not a security fix, but since LeaseState drives client
caching decisions, it's probably worth closing.  Reading lcontext
from DataOffset, as with DiskFileId would make the two handlers
consistent.

(My version also had this, so it's more an observation than anything
else)


2) You may want to consider matching DataLength exactly, rather than
   taking a minimum.

ksmbd's parse_lease_state() requires:

	sizeof(struct lease_context_v2) == le32_to_cpu(cc->DataLength)

and validates DataOffset + DataLength against the create_lease_v2
size, rather than accepting anything at least long enough.  It was
suggested to me that having the client & server halves agree on
strictness would be good.

(I did confirm your minimums cover all the fields each of the parsers
actually touch, so this is about strictness, not a hole)


Frank
--
Frank Sorenson
sorenson@redhat.com
Principal Software Maintenance Engineer, filesystems
Red Hat

On Sat, Sep 12, 2026 at 1:05 AM Frank Sorenson [off-list ref] wrote:

On Fri, Sep 04, 2026 at 02:05:17PM +0000, Zihan Xi wrote:

quoted

smb2_parse_contexts() validates the complete create-context area but does
not bound each context record by its Next field before dispatching to a
handler.  A malformed chain can therefore expose bytes past one context to
the handler.  The QFid handler also used a full response-structure cast even
though it only consumes DiskFileId.

Hello,

I have some thoughts/comments on your series.

First, a possible reason for not getting a response on your v2, and
delayed response on this v3: Steve French passed away in August, so
mail addressed to him isn't reaching a maintaner, and things may
have gotten missed during transitions.  You'll want to send future
versions to Paulo Alcantara [off-list ref], with Cc to
Namjae Jeon [off-list ref]


I've been carrying an overlapping patch (an earlier posting:
https://lore.kernel.org/r/20260826153147.4112943-12-sorenson@redhat.com).
in a bounds-checking series of my own.  Like yours, my patch has
been addressing the memory safety problem in the three handlers
which read at fixed offsets, rather than the generic checks.

But I think yours is probably a better fix, and you've got the
PoC, so if we can perfect yours and get it in, I'll be dropping
mine in favor of this series.


A few points (take with a grain of salt):

1) The lease parser still reads at a fixed offset rather than from
   DataOffset:

quoted

              case 4:
                      if (!strncmp(name, SMB2_CREATE_REQUEST_LEASE, 4)) {
-                             *oplock = server->ops->parse_lease_buf(cc, epoch,
+                             if (cc_len >= smb2_create_lease_min_cc_len(server))

40 more quoted lines

Hi Frank,

Thank you for the review.  I'm sorry to hear about Steve.  I'll send
future versions to Paulo Alcantara, with Cc to Namjae Jeon.

I also appreciate you carrying the overlapping bounds-checking series.

I'll address both comments in v4 and send it as a new thread:

1) smb2_parse_lease_buf() and smb3_parse_lease_buf() will copy the
   lease context from DataOffset, as the QFid handler does for
   DiskFileId.  Agreed this is not a security fix; it just keeps
   LeaseState correct when DataOffset is non-canonical.

2) DataLength will be required to equal sizeof(struct lease_context)
   or sizeof(struct lease_context_v2) for the corresponding parser,
   rather than a minimum.  A mismatch skips lease parsing and does
   not fail the open.

Thanks,
Zihan