* [PATCH v4 1/6] smb: client: fix create context out-of-bounds reads
2026-09-16 15:29 [PATCH v4 0/6] smb: client: fix create context out-of-bounds reads Zihan Xi
@ 2026-09-16 15:29 ` Zihan Xi
2026-09-16 15:29 ` [PATCH v4 2/6] smb: client: validate POSIX create context length Zihan Xi
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Zihan Xi @ 2026-09-16 15:29 UTC (permalink / raw)
To: pc; +Cc: linkinjeon, zihanx, linux-cifs, samba-technical
smb2_parse_contexts() validates the complete create-context area but
does not limit each record to its Next field before dispatching it. A
malformed chain can therefore expose bytes beyond the current context to
a handler. The QFid handler also used a full response-structure cast
although it only reads DiskFileId.
The SMB2/SMB3 lease parsers made the same layout assumption: they read
LeaseState and LeaseFlags at canonical offsets rather than at
DataOffset. A valid non-canonical DataOffset could therefore yield
unrelated in-bounds data, while a short DataLength was still accepted.
Limit each context to its Next value, reject offsets before the context
header, and reject malformed chains. Bound the name range by the current
context and do not dispatch a known handler when DataLength is zero. Read
the QFid DiskFileId only when the context data covers that field. Parse the
lease context from DataOffset and require DataLength to match the v1 or v2
lease_context size used by ksmbd. A size mismatch skips lease parsing
without failing the open.
Fixes: b8c32dbb0deb ("CIFS: Request SMB2.1 leases")
Fixes: f047390a097e ("CIFS: Add create lease v2 context for SMB3")
Fixes: 89a5bfa350fa ("smb3: optimize open to not send query file internal info")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: LLM
Co-developed-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
---
changes in v4:
- Reject NameOffset and DataOffset values before the context header and
use checked arithmetic for the name range.
- Bound the name range by the current record and skip known-handler
dispatch when DataLength is zero.
- Parse lease data from DataOffset and require exact v1/v2 payload sizes.
- Add the SMB3 lease v2 Fixes attribution for f047390a097e.
- v3 Link: https://lore.kernel.org/all/cover.1788516372.git.zihanx@nebusec.ai/
changes in v3:
- Split the POSIX handler check into a separate patch and corrected the
parser Fixes history.
- 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.
- Add a POSIX handler check for the three fixed fields.
- v1 Link: https://lore.kernel.org/all/eb1bc35611f91bd10a4772400b37fac26f660956.1782579150.git.xizh2024@lzu.edu.cn/
---
fs/smb/client/smb2ops.c | 28 ++++++++++++++++++--------
fs/smb/client/smb2pdu.c | 44 +++++++++++++++++++++++++++++++----------
2 files changed, 54 insertions(+), 18 deletions(-)
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 192649fec25d5..749cee88fc38d 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -4437,25 +4437,37 @@ smb3_create_lease_buf(u8 *lease_key, u8 oplock, u8 *parent_lease_key, __le32 fla
static __u8
smb2_parse_lease_buf(void *buf, __u16 *epoch, char *lease_key)
{
- struct create_lease *lc = (struct create_lease *)buf;
+ struct create_context *cc = buf;
+ struct lease_context lc;
*epoch = 0; /* not used */
- if (lc->lcontext.LeaseFlags & SMB2_LEASE_FLAG_BREAK_IN_PROGRESS_LE)
+ if (le32_to_cpu(cc->DataLength) != sizeof(lc))
+ return 0;
+
+ memcpy(&lc, (u8 *)cc + le16_to_cpu(cc->DataOffset), sizeof(lc));
+ if (lc.LeaseFlags & SMB2_LEASE_FLAG_BREAK_IN_PROGRESS_LE)
return SMB2_OPLOCK_LEVEL_NOCHANGE;
- return le32_to_cpu(lc->lcontext.LeaseState);
+ return le32_to_cpu(lc.LeaseState);
}
static __u8
smb3_parse_lease_buf(void *buf, __u16 *epoch, char *lease_key)
{
- struct create_lease_v2 *lc = (struct create_lease_v2 *)buf;
+ struct create_context *cc = buf;
+ struct lease_context_v2 lc;
+
+ if (le32_to_cpu(cc->DataLength) != sizeof(lc)) {
+ *epoch = 0;
+ return 0;
+ }
- *epoch = le16_to_cpu(lc->lcontext.Epoch);
- if (lc->lcontext.LeaseFlags & SMB2_LEASE_FLAG_BREAK_IN_PROGRESS_LE)
+ memcpy(&lc, (u8 *)cc + le16_to_cpu(cc->DataOffset), sizeof(lc));
+ *epoch = le16_to_cpu(lc.Epoch);
+ if (lc.LeaseFlags & SMB2_LEASE_FLAG_BREAK_IN_PROGRESS_LE)
return SMB2_OPLOCK_LEVEL_NOCHANGE;
if (lease_key)
- memcpy(lease_key, &lc->lcontext.LeaseKey, SMB2_LEASE_KEY_SIZE);
- return le32_to_cpu(lc->lcontext.LeaseState);
+ memcpy(lease_key, lc.LeaseKey, SMB2_LEASE_KEY_SIZE);
+ return le32_to_cpu(lc.LeaseState);
}
static unsigned int
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 4ce165e40657f..7a6627400ba30 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -2378,11 +2378,17 @@ create_reconnect_durable_buf(struct cifs_fid *fid)
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;
- cifs_dbg(FYI, "parse query id context 0x%llx 0x%llx\n",
- pdisk_id->DiskFileId, pdisk_id->VolumeId);
- buf->IndexNumber = pdisk_id->DiskFileId;
+ if (dlen < sizeof(__le64))
+ return;
+
+ 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 +2436,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,29 +2459,41 @@ 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 (doff < sizeof(*cc) ||
+ check_add_overflow(doff, dlen, &len) || len > cc_len)
return -EINVAL;
noff = le16_to_cpu(cc->NameOffset);
nlen = le16_to_cpu(cc->NameLength);
- if (noff + nlen > doff)
+ if (noff < sizeof(*cc) ||
+ check_add_overflow(noff, nlen, &len) || len > cc_len ||
+ (dlen && len > doff))
return -EINVAL;
name = (char *)cc + noff;
switch (nlen) {
case 4:
- if (!strncmp(name, SMB2_CREATE_REQUEST_LEASE, 4)) {
+ if (dlen && !strncmp(name, SMB2_CREATE_REQUEST_LEASE, 4)) {
*oplock = server->ops->parse_lease_buf(cc, epoch,
lease_key);
- } else if (buf &&
+ } else if (dlen && buf &&
!strncmp(name, SMB2_CREATE_QUERY_ON_DISK_ID, 4)) {
parse_query_id_ctxt(cc, buf);
}
break;
case 16:
- if (posix && !memcmp(name, smb3_create_tag_posix, 16))
+ if (dlen && posix && !memcmp(name, smb3_create_tag_posix, 16))
parse_posix_ctxt(cc, buf, posix);
break;
default:
@@ -2486,13 +2505,18 @@ int smb2_parse_contexts(struct TCP_Server_Info *server,
}
off = le32_to_cpu(cc->Next);
- if (!off)
+ if (!off) {
+ rem = 0;
break;
+ }
if (check_sub_overflow(rem, off, &rem))
return -EINVAL;
cc = (struct create_context *)((u8 *)cc + off);
}
+ if (rem)
+ return -EINVAL;
+
if (rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE)
*oplock = rsp->OplockLevel;
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v4 2/6] smb: client: validate POSIX create context length
2026-09-16 15:29 [PATCH v4 0/6] smb: client: fix create context out-of-bounds reads Zihan Xi
2026-09-16 15:29 ` [PATCH v4 1/6] " Zihan Xi
@ 2026-09-16 15:29 ` Zihan Xi
2026-09-16 15:29 ` [PATCH v4 3/6] smb: client: close handle after create-context parsing failure Zihan Xi
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Zihan Xi @ 2026-09-16 15:29 UTC (permalink / raw)
To: pc; +Cc: linkinjeon, zihanx, linux-cifs, samba-technical
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 checks and still make these fixed-width
reads run past its declared data.
The current in-tree smb2_open_file() path passes a NULL posix pointer,
so this handler is not reached on the ordinary open path. Still require
the POSIX data to cover all three fields before reading them because the
helper performs those unguarded reads. Keep the existing soft-failure
behavior so malformed optional metadata does not fail the open.
Fixes: 69dda3059e7a ("cifs: add SMB2_open() arg to return POSIX data")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: LLM
Co-developed-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
---
changes in v4:
- Keep the handler-level minimum check and preserve soft failure for
malformed optional POSIX metadata.
- v3 Link: https://lore.kernel.org/all/cover.1788516372.git.zihanx@nebusec.ai/
changes in v3:
- Split the POSIX handler check into a separate patch and corrected the
parser Fixes history.
- v2 Link: https://lore.kernel.org/all/cover.1787486936.git.zihanx@nebusec.ai/
changes in v2:
- Add the POSIX handler check for the three fixed fields.
- v1 Link: https://lore.kernel.org/all/eb1bc35611f91bd10a4772400b37fac26f660956.1782579150.git.xizh2024@lzu.edu.cn/
---
fs/smb/client/smb2pdu.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 7a6627400ba30..1b2ca3b2c2f87 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -2395,12 +2395,15 @@ static void
parse_posix_ctxt(struct create_context *cc, struct smb2_file_all_info *info,
struct create_posix_rsp *posix)
{
- int sid_len;
u8 *beg = (u8 *)cc + le16_to_cpu(cc->DataOffset);
- u8 *end = beg + le32_to_cpu(cc->DataLength);
+ u32 dlen = le32_to_cpu(cc->DataLength);
+ u8 *end = beg + dlen;
+ int sid_len;
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v4 3/6] smb: client: close handle after create-context parsing failure
2026-09-16 15:29 [PATCH v4 0/6] smb: client: fix create context out-of-bounds reads Zihan Xi
2026-09-16 15:29 ` [PATCH v4 1/6] " Zihan Xi
2026-09-16 15:29 ` [PATCH v4 2/6] smb: client: validate POSIX create context length Zihan Xi
@ 2026-09-16 15:29 ` Zihan Xi
2026-09-16 15:29 ` [PATCH v4 4/6] smb: client: clean up failed cached directory opens Zihan Xi
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Zihan Xi @ 2026-09-16 15:29 UTC (permalink / raw)
To: pc; +Cc: linkinjeon, zihanx, linux-cifs, samba-technical
SMB2_open() accounts a successful CREATE response as a remote open before
parsing its create contexts. If smb2_parse_contexts() rejects malformed
context data, SMB2_open() returns without closing the handle, leaving the
server-side handle open and num_remote_opens elevated.
Close the handle after a post-CREATE context parsing failure so the error
path releases the remote resource and balances the open count.
Fixes: af1689a9b770 ("smb: client: fix potential OOBs in smb2_parse_contexts()")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: LLM
Co-developed-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
---
changes in v4:
- Keep post-CREATE cleanup for parser failures and document the existing
best-effort close behavior.
- v3 Link: https://lore.kernel.org/all/cover.1788516372.git.zihanx@nebusec.ai/
changes in v3:
- Split the POSIX handler check into a separate patch and corrected the
parser Fixes history.
- v2 Link: https://lore.kernel.org/all/cover.1787486936.git.zihanx@nebusec.ai/
changes in v2:
- Add cleanup after a create-context parsing failure.
- v1 Link: https://lore.kernel.org/all/eb1bc35611f91bd10a4772400b37fac26f660956.1782579150.git.xizh2024@lzu.edu.cn/
---
fs/smb/client/smb2pdu.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 1b2ca3b2c2f87..12973f2e26b16 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -3414,6 +3414,9 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
rc = smb2_parse_contexts(server, &rsp_iov, &oparms->fid->epoch,
oparms->fid->lease_key, oplock, file_info, posix);
+ if (rc)
+ SMB2_close(xid, tcon, oparms->fid->persistent_fid,
+ oparms->fid->volatile_fid);
trace_smb3_open_done(xid, rsp->PersistentFileId, tcon->tid, ses->Suid,
oparms->create_options, oparms->desired_access,
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v4 4/6] smb: client: clean up failed cached directory opens
2026-09-16 15:29 [PATCH v4 0/6] smb: client: fix create context out-of-bounds reads Zihan Xi
` (2 preceding siblings ...)
2026-09-16 15:29 ` [PATCH v4 3/6] smb: client: close handle after create-context parsing failure Zihan Xi
@ 2026-09-16 15:29 ` Zihan Xi
2026-09-16 15:29 ` [PATCH v4 5/6] smb: client: close completed creates on compound wait errors Zihan Xi
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Zihan Xi @ 2026-09-16 15:29 UTC (permalink / raw)
To: pc; +Cc: linkinjeon, zihanx, linux-cifs, samba-technical
open_cached_dir() sends CREATE and QUERY_INFO as a compound request. If
the CREATE succeeds but a later command returns an error, the function
must retain the CREATE FID so common cleanup can issue SMB2_close(). It
also must not treat a response error as a valid CREATE.
Validate the CREATE response before using its fields, record the FIDs, and
mark the handle open before handling errors from later compound commands.
Move the -EREMCHG reconnect handling before response validation so a
missing response does not hide the reconnect request. Count the handle
when it is marked open; confirmed close responses decrement the counter,
while existing close retry behavior remains best effort on transport
failures.
Fixes: b0f6df737a1c ("cifs: cache FILE_ALL_INFO for the shared root handle")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: LLM
Co-developed-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
---
changes in v4:
- Validate CREATE status and FIDs before marking the cached-directory handle
open, preserve reconnect handling, and balance cleanup accounting.
- Keep the cached-directory Fixes attribution at b0f6df737a1c.
- v3 Link: https://lore.kernel.org/all/cover.1788516372.git.zihanx@nebusec.ai/
changes in v3:
- Split the POSIX handler check into a separate patch and corrected the
parser Fixes history.
- v2 Link: https://lore.kernel.org/all/cover.1787486936.git.zihanx@nebusec.ai/
changes in v2:
- Add cached-directory cleanup for failed compound opens.
- v1 Link: https://lore.kernel.org/all/eb1bc35611f91bd10a4772400b37fac26f660956.1782579150.git.xizh2024@lzu.edu.cn/
---
fs/smb/client/cached_dir.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
index 88d5e9a32f28b..647fa26da4d24 100644
--- a/fs/smb/client/cached_dir.c
+++ b/fs/smb/client/cached_dir.c
@@ -8,6 +8,7 @@
#include <linux/namei.h>
#include "cifsglob.h"
#include "cifsproto.h"
+#include "../common/smb2status.h"
#include "cifs_debug.h"
#include "smb2proto.h"
#include "cached_dir.h"
@@ -323,25 +324,37 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
rc = compound_send_recv(xid, ses, server,
flags, 2, rqst,
resp_buftype, rsp_iov);
- if (rc) {
- if (rc == -EREMCHG) {
- tcon->need_reconnect = true;
- pr_warn_once("server share %s deleted\n",
- tcon->tree_name);
- }
- goto oshr_free;
+ if (rc == -EREMCHG) {
+ tcon->need_reconnect = true;
+ pr_warn_once("server share %s deleted\n",
+ tcon->tree_name);
}
- cfid->is_open = true;
- spin_lock(&cfids->cfid_list_lock);
+ if (!rsp_iov[0].iov_base || rsp_iov[0].iov_len < sizeof(*o_rsp)) {
+ if (!rc)
+ rc = -EIO;
+ goto oshr_free;
+ }
o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base;
+ if (o_rsp->hdr.Status != STATUS_SUCCESS) {
+ if (!rc)
+ rc = -EIO;
+ goto oshr_free;
+ }
+
oparms.fid->persistent_fid = o_rsp->PersistentFileId;
oparms.fid->volatile_fid = o_rsp->VolatileFileId;
#ifdef CONFIG_CIFS_DEBUG2
oparms.fid->mid = le64_to_cpu(o_rsp->hdr.MessageId);
#endif /* CIFS_DEBUG2 */
+ cfid->is_open = true;
+ atomic_inc(&tcon->num_remote_opens);
+ if (rc)
+ goto oshr_free;
+
+ spin_lock(&cfids->cfid_list_lock);
if (o_rsp->OplockLevel != SMB2_OPLOCK_LEVEL_LEASE) {
spin_unlock(&cfids->cfid_list_lock);
@@ -408,7 +421,6 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
close_cached_dir(cfid);
} else {
*ret_cfid = cfid;
- atomic_inc(&tcon->num_remote_opens);
}
kfree(utf16_path);
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v4 5/6] smb: client: close completed creates on compound wait errors
2026-09-16 15:29 [PATCH v4 0/6] smb: client: fix create context out-of-bounds reads Zihan Xi
` (3 preceding siblings ...)
2026-09-16 15:29 ` [PATCH v4 4/6] smb: client: clean up failed cached directory opens Zihan Xi
@ 2026-09-16 15:29 ` Zihan Xi
2026-09-16 15:29 ` [PATCH v4 6/6] smb: client: preserve create-context parsing errors Zihan Xi
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Zihan Xi @ 2026-09-16 15:29 UTC (permalink / raw)
To: pc; +Cc: linkinjeon, zihanx, linux-cifs, samba-technical
compound_send_recv() waits for responses in order. If a later wait is
interrupted, or if a later MID fails during response synchronization, an
earlier CREATE may already have opened a remote handle. The earlier mid
is then released without invoking handle_cancelled_mid(), leaving the
remote handle open because no FID was copied to the caller.
Mark completed earlier mids as cancelled when a compound wait or MID
synchronization aborts. Keep their response buffers attached while the
MIDs are synchronized, and transfer them only after synchronization of
the processed responses, so the release path can inspect successful
CREATE responses and queue SMB2_close() after a later failure. Account for
a remote open only after the close work is allocated and before it is
queued, since the caller has not yet updated num_remote_opens. Mark the
create+close compound used by smb2_unlink() so it is not closed again.
Non-CREATE responses and compounds that already include a close keep their
existing behavior.
Fixes: e0bba0b85481 ("cifs: add compound_send_recv()")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: LLM
Co-developed-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
---
changes in v4:
- Keep response buffers attached while MIDs are synchronized so a later
MID failure can trigger cancelled-mid cleanup for earlier CREATEs.
- Cover MID synchronization and unready-state failures, and defer
num_remote_opens accounting until close work allocation succeeds.
- Mark smb2_unlink()'s create+close compound to avoid duplicate cleanup.
- Correct the Fixes tag to e0bba0b85481.
- v3 Link: https://lore.kernel.org/all/cover.1788516372.git.zihanx@nebusec.ai/
changes in v3:
- Add cleanup for completed CREATEs when a compound wait is interrupted.
- v2 Link: https://lore.kernel.org/all/cover.1787486936.git.zihanx@nebusec.ai/
changes in v2:
- No counterpart in v2.
- v1 Link: https://lore.kernel.org/all/eb1bc35611f91bd10a4772400b37fac26f660956.1782579150.git.xizh2024@lzu.edu.cn/
---
fs/smb/client/smb2inode.c | 2 +-
fs/smb/client/smb2misc.c | 9 ++++--
fs/smb/client/transport.c | 67 +++++++++++++++++++++++++++++++--------
3 files changed, 61 insertions(+), 17 deletions(-)
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 213bc298cdf22..6971496dfe8c2 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -1104,7 +1104,7 @@ smb2_unlink(const unsigned int xid, struct cifs_tcon *tcon, const char *name,
struct kvec close_iov;
int resp_buftype[2];
struct cifs_fid fid;
- int flags = 0;
+ int flags = CIFS_CP_CREATE_CLOSE_OP;
__u8 oplock;
int rc;
diff --git a/fs/smb/client/smb2misc.c b/fs/smb/client/smb2misc.c
index 9068175e57cd0..596388acb31e7 100644
--- a/fs/smb/client/smb2misc.c
+++ b/fs/smb/client/smb2misc.c
@@ -821,7 +821,8 @@ smb2_cancelled_close_fid(struct work_struct *work)
*/
static int
__smb2_handle_cancelled_cmd(struct cifs_tcon *tcon, __u16 cmd, __u64 mid,
- __u64 persistent_fid, __u64 volatile_fid)
+ __u64 persistent_fid, __u64 volatile_fid,
+ bool account_remote_open)
{
struct close_cancelled_open *cancelled;
@@ -835,6 +836,8 @@ __smb2_handle_cancelled_cmd(struct cifs_tcon *tcon, __u16 cmd, __u64 mid,
cancelled->cmd = cmd;
cancelled->mid = mid;
INIT_WORK(&cancelled->work, smb2_cancelled_close_fid);
+ if (account_remote_open)
+ atomic_inc(&tcon->num_remote_opens);
WARN_ON(queue_work(cifsiod_wq, &cancelled->work) == false);
return 0;
@@ -871,7 +874,7 @@ smb2_handle_cancelled_close(struct cifs_tcon *tcon, __u64 persistent_fid,
spin_unlock(&tcon->tc_lock);
rc = __smb2_handle_cancelled_cmd(tcon, SMB2_CLOSE_HE, 0,
- persistent_fid, volatile_fid);
+ persistent_fid, volatile_fid, false);
if (rc)
cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_cancelled_close);
@@ -899,7 +902,7 @@ smb2_handle_cancelled_mid(struct mid_q_entry *mid, struct TCP_Server_Info *serve
le16_to_cpu(hdr->Command),
le64_to_cpu(hdr->MessageId),
rsp->PersistentFileId,
- rsp->VolatileFileId);
+ rsp->VolatileFileId, true);
if (rc)
cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_cancelled_mid);
diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
index fdf4e50c27ceb..14ba8646e5625 100644
--- a/fs/smb/client/transport.c
+++ b/fs/smb/client/transport.c
@@ -806,6 +806,18 @@ cifs_cancelled_callback(struct TCP_Server_Info *server, struct mid_q_entry *mid)
release_mid(server, mid);
}
+static void
+cifs_mark_compound_mids_cancelled(struct mid_q_entry **mid, int count)
+{
+ int i;
+
+ for (i = 0; i < count; i++) {
+ spin_lock(&mid[i]->mid_lock);
+ mid[i]->wait_cancelled = true;
+ spin_unlock(&mid[i]->mid_lock);
+ }
+}
+
/*
* cifs_pick_channel - pick an eligible channel for network operations
*
@@ -866,6 +878,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
int *resp_buf_type, struct kvec *resp_iov)
{
int i, j, optype, rc = 0;
+ int num_processed = 0;
struct mid_q_entry *mid[MAX_COMPOUND];
bool cancelled_mid[MAX_COMPOUND] = {false};
struct cifs_credits credits[MAX_COMPOUND] = {
@@ -1012,6 +1025,14 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
break;
}
if (rc != 0) {
+ /*
+ * A completed CREATE earlier in the compound chain may have
+ * opened a remote handle even though a later wait was
+ * interrupted. Mark it cancelled so __release_mid() invokes
+ * the existing unmatched-open cleanup.
+ */
+ cifs_mark_compound_mids_cancelled(mid, i);
+
for (; i < num_rqst; i++) {
cifs_server_dbg(FYI, "Cancelling wait for mid %llu cmd: %d\n",
mid[i]->mid, le16_to_cpu(mid[i]->command));
@@ -1034,6 +1055,14 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
rc = cifs_sync_mid_result(mid[i], server);
if (rc != 0) {
+ /*
+ * A previous CREATE may have completed before this
+ * response failed. Mark it cancelled so its remote
+ * handle is closed when the mid is released.
+ */
+ cifs_mark_compound_mids_cancelled(mid, i);
+ /* Keep their response buffers for cancelled-mid cleanup. */
+ num_processed = 0;
/* mark this mid as cancelled to not free it below */
cancelled_mid[i] = true;
goto out;
@@ -1043,13 +1072,24 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
mid[i]->mid_state != MID_RESPONSE_READY) {
rc = smb_EIO1(smb_eio_trace_rx_mid_unready, mid[i]->mid_state);
cifs_dbg(FYI, "Bad MID state?\n");
+ cifs_mark_compound_mids_cancelled(mid, i);
+ num_processed = 0;
goto out;
}
rc = server->ops->check_receive(mid[i], server,
flags & CIFS_LOG_ERROR);
+ num_processed = i + 1;
+ }
- if (resp_iov) {
+out:
+ /*
+ * Delay moving response buffers out of their mids until response
+ * synchronization completes. This lets cancelled-mid cleanup inspect
+ * an earlier CREATE response if a later MID fails.
+ */
+ if (resp_iov) {
+ for (i = 0; i < num_processed; i++) {
buf = (char *)mid[i]->resp_buf;
resp_iov[i].iov_base = buf;
resp_iov[i].iov_len = mid[i]->resp_buf_size;
@@ -1068,21 +1108,22 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
/*
* Compounding is never used during session establish.
*/
- spin_lock(&ses->ses_lock);
- if ((ses->ses_status == SES_NEW) || (optype & CIFS_NEG_OP) || (optype & CIFS_SESS_OP)) {
- struct kvec iov = {
- .iov_base = resp_iov[0].iov_base,
- .iov_len = resp_iov[0].iov_len
- };
- spin_unlock(&ses->ses_lock);
- cifs_server_lock(server);
- smb311_update_preauth_hash(ses, server, &iov, 1);
- cifs_server_unlock(server);
+ if (num_processed == num_rqst) {
spin_lock(&ses->ses_lock);
+ if ((ses->ses_status == SES_NEW) || (optype & CIFS_NEG_OP) || (optype & CIFS_SESS_OP)) {
+ struct kvec iov = {
+ .iov_base = resp_iov[0].iov_base,
+ .iov_len = resp_iov[0].iov_len
+ };
+ spin_unlock(&ses->ses_lock);
+ cifs_server_lock(server);
+ smb311_update_preauth_hash(ses, server, &iov, 1);
+ cifs_server_unlock(server);
+ spin_lock(&ses->ses_lock);
+ }
+ spin_unlock(&ses->ses_lock);
}
- spin_unlock(&ses->ses_lock);
-out:
/*
* This will dequeue all mids. After this it is important that the
* demultiplex_thread will not process any of these mids any further.
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v4 6/6] smb: client: preserve create-context parsing errors
2026-09-16 15:29 [PATCH v4 0/6] smb: client: fix create context out-of-bounds reads Zihan Xi
` (4 preceding siblings ...)
2026-09-16 15:29 ` [PATCH v4 5/6] smb: client: close completed creates on compound wait errors Zihan Xi
@ 2026-09-16 15:29 ` Zihan Xi
2026-09-21 15:10 ` [PATCH v4 0/6] smb: client: fix create context out-of-bounds reads Frank Sorenson
2026-09-23 1:09 ` Paulo Alcantara
7 siblings, 0 replies; 9+ messages in thread
From: Zihan Xi @ 2026-09-16 15:29 UTC (permalink / raw)
To: pc; +Cc: linkinjeon, zihanx, linux-cifs, samba-technical
smb2_compound_op() saves the result from compound_send_recv() in
tmp_rc. For SMB2_OP_OPEN_QUERY it then parses the CREATE contexts, but
the final assignment of rc from tmp_rc discards a parsing error. A
malformed create-context response can therefore be reported as
successful to smb2_query_path_info().
Keep a create-context parsing error in tmp_rc so it survives per-command
response processing and is returned to the caller.
Fixes: b07687edee99 ("cifs: Improve SMB2+ stat() to work also without FILE_READ_ATTRIBUTES")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: LLM
Co-developed-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Luxing Yin <root@tr0jan.top>
Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
---
changes in v4:
- Keep a create-context parsing error in tmp_rc while processing later
compound responses.
- v3 Link: https://lore.kernel.org/all/cover.1788516372.git.zihanx@nebusec.ai/
changes in v3:
- No counterpart; this patch is added in the v4 reroll.
- v2 Link: https://lore.kernel.org/all/cover.1787486936.git.zihanx@nebusec.ai/
changes in v2:
- No counterpart in v2.
- v1 Link: https://lore.kernel.org/all/eb1bc35611f91bd10a4772400b37fac26f660956.1782579150.git.xizh2024@lzu.edu.cn/
---
fs/smb/client/smb2inode.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 6971496dfe8c2..cedad9daeb220 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -582,8 +582,10 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
/* smb2_parse_contexts() fills idata->fi.IndexNumber */
rc = smb2_parse_contexts(server, &rsp_iov[0], &oparms->fid->epoch,
oparms->fid->lease_key, &oplock, &idata->fi, NULL);
- if (rc)
+ if (rc) {
cifs_dbg(VFS, "rc: %d parsing context of compound op\n", rc);
+ tmp_rc = rc;
+ }
}
for (i = 0; i < num_cmds; i++) {
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v4 0/6] smb: client: fix create context out-of-bounds reads
2026-09-16 15:29 [PATCH v4 0/6] smb: client: fix create context out-of-bounds reads Zihan Xi
` (5 preceding siblings ...)
2026-09-16 15:29 ` [PATCH v4 6/6] smb: client: preserve create-context parsing errors Zihan Xi
@ 2026-09-21 15:10 ` Frank Sorenson
2026-09-23 1:09 ` Paulo Alcantara
7 siblings, 0 replies; 9+ messages in thread
From: Frank Sorenson @ 2026-09-21 15:10 UTC (permalink / raw)
To: Zihan Xi, pc; +Cc: linkinjeon, linux-cifs, samba-technical
On 9/16/26 10:29 AM, Zihan Xi wrote:
> Hi Linux kernel maintainers,
>
> We found and validated an issue in fs/smb/client/smb2pdu.c. A malicious
> SMB server can send a malformed SMB2 CREATE response to a CIFS client and
> trigger an out-of-bounds read. We validated this with an Impacket server on
> the QEMU host and a root CIFS client in the guest; CIFS does not set
> FS_USERNS_MOUNT, so the mount must run as root. For valid SMB responses,
> the series preserves existing request handling. No impact was observed in
> the tested CIFS mount/read path; a full filesystem regression suite was not
> run.
I ran xfstests comparing cifs-next (7.3-rc2+) with and without this patchset,
and I don't see any additional failures due with these patches:
tested against:
samba: v3.1.1, v3.1.1+sign, v3.1.1+seal, v2.1, v2.1+sign, v1
Windows Server 2022: v3.1.1, v3.1.1+sign, v3.1.1+seal
I have not tested using the POC... just for regressions.
Tested-by: Frank Sorenson <sorenson@redhat.com>
Frank
--
Frank Sorenson
sorenson@redhat.com
Principal Software Maintenance Engineer, filesystems
Red Hat
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v4 0/6] smb: client: fix create context out-of-bounds reads
2026-09-16 15:29 [PATCH v4 0/6] smb: client: fix create context out-of-bounds reads Zihan Xi
` (6 preceding siblings ...)
2026-09-21 15:10 ` [PATCH v4 0/6] smb: client: fix create context out-of-bounds reads Frank Sorenson
@ 2026-09-23 1:09 ` Paulo Alcantara
7 siblings, 0 replies; 9+ messages in thread
From: Paulo Alcantara @ 2026-09-23 1:09 UTC (permalink / raw)
To: Zihan Xi; +Cc: linkinjeon, zihanx, linux-cifs, samba-technical
Zihan Xi <zihanx@nebusec.ai> writes:
> Hi Linux kernel maintainers,
>
> We found and validated an issue in fs/smb/client/smb2pdu.c. A malicious
> SMB server can send a malformed SMB2 CREATE response to a CIFS client and
> trigger an out-of-bounds read. We validated this with an Impacket server on
> the QEMU host and a root CIFS client in the guest; CIFS does not set
> FS_USERNS_MOUNT, so the mount must run as root. For valid SMB responses,
> the series preserves existing request handling. No impact was observed in
> the tested CIFS mount/read path; a full filesystem regression suite was not
> run.
>
> We will provide detailed information about the bug
> in this email, along with a PoC to trigger it.
>
> ---- details below ----
>
> Bug details:
>
> smb2_parse_contexts() validates the complete create-context area but does
> not limit each context record to its Next field before dispatching it. A
> malformed chain can therefore allow a handler to read bytes beyond the
> current context. The QFid handler also cast the context to a full response
> structure without verifying that DataLength covered DiskFileId, so a
> truncated QFid context could read past the response allocation. A
> non-terminal Next that does not leave a complete following context header
> is rejected as malformed.
>
> The parser rejects NameOffset and DataOffset values before the context
> header, bounds the name range by the current record with checked arithmetic,
> and dispatches known handlers only when DataLength is non-zero.
>
> The SMB2/SMB3 lease parsers also read LeaseState and LeaseFlags at
> canonical offsets rather than from DataOffset. Patch 1 limits each record
> to Next, reads QFid data only when its payload covers DiskFileId, and
> parses lease data from DataOffset with the exact v1/v2 lease payload sizes.
> A size mismatch skips lease parsing without failing the open. These sizes
> match the fixed payload sizes used by the CIFS request builders and
> ksmbd; a future extension must update the parser explicitly.
>
> parse_posix_ctxt() reads nlink, reparse_tag, and mode before checking that
> the POSIX data contains them. The in-tree smb2_open_file() path passes a
> NULL posix pointer, so ordinary opens do not reach this handler. Patch 2
> still checks the handler's minimum data length and preserves soft failure
> for malformed optional metadata.
>
> Tracing the parser callers and compound error paths through cleanup and
> return-value handling also exposed the additional independent issues fixed
> by patches 3 through 6.
>
> After a successful CREATE, SMB2_open() increments num_remote_opens before
> parsing its contexts. Patch 3 calls SMB2_close() after a parsing failure.
> SMB2_close() decrements num_remote_opens only after a confirmed successful
> close response. If a close is interrupted or transport fails, the existing
> best-effort behavior retains conservative accounting when the remote result
> is unknown.
>
> open_cached_dir() sends CREATE and QUERY_INFO as a compound request. Patch
> 4 validates the CREATE response before using its fields, records the CREATE
> FIDs, marks the handle open, and increments the remote-open count before
> processing later-command errors. It also handles -EREMCHG before response
> validation, so a missing response does not hide the reconnect request.
> Patch 5 marks earlier completed mids as cancelled when a later compound
> wait is interrupted or MID synchronization or state validation fails. It
> keeps their response buffers attached until synchronization is complete, so
> the existing cancelled-mid cleanup can inspect each successful CREATE and
> queue SMB2_close(). The remote-open count is incremented only after close
> work allocation succeeds and before queueing it, so an OOM does not leave
> an unmatched count. It marks smb2_unlink()'s create+close compound so it
> is not closed again. Non-CREATE responses and compounds with a close keep
> their existing behavior.
>
> Patch 6 preserves a create-context parsing error in the
> SMB2_OP_OPEN_QUERY compound path while later responses are processed.
>
> The series keeps separate Fixes tags for the independent root causes, with
> each tag pointing to the earliest commit that introduced its root cause.
>
> The parser changes overlap with Frank Sorenson's related bounds-checking
> patch for smb2_parse_contexts():
> https://lore.kernel.org/all/20260826153147.4112943-12-sorenson@redhat.com/
> This series incorporates the NameOffset, Next, and zero-data dispatch checks
> while retaining the stricter per-record successor-header validation and the
> handler-specific payload checks.
>
> The reproducer uses an Impacket SMB server that modifies the SMB2 CREATE
> response. packetdrill is not used because it cannot implement the required
> stateful SMB server or rewrite this response.
> ...
Applied.
^ permalink raw reply [flat|nested] 9+ messages in thread