[PATCH 00/11] smb: client: fix OOB reads and UAFs in SMB2/3 receive paths

[PATCH 00/11] smb: client: fix OOB reads and UAFs in SMB2/3 receive paths

[Frank Sorenson]

This series fixes eleven bounds-checking defects in the SMB2/3 client,
all reachable from a malicious or compromised server.

Patches 1-3 address the compound encrypted frame processing path:

Patch 1 fixes four interacting bugs in receive_encrypted_standard():
a missing lower bound on NextCommand, an off-by-one upper bound that
admitted trailing slices too small for an SMB2 header (producing a
write-after-free via next_buffer aliasing server->bigbuf), a stale
next_buffer pointer not cleared before goto one_more, and use of the
pre-decryption pdu_length instead of the plaintext extent for bounds
checking.

Patch 2 adds smb2_min_pdu_len[], a per-command table of minimum response
struct sizes, and uses it in smb2_check_message() to reject responses
too short for smb2_get_data_area_len() to safely read command-specific
struct fields.

Patch 3 fixes server->total_read tracking in receive_encrypted_standard()
so that smb2_check_message() sees the actual per-sub-PDU size rather than
the full remaining compound tail.  Without this, a rogue server can craft
a compound frame where any non-last sub-PDU is shorter than its declared
fixed struct, bypassing the guards added in patch 2.

The remaining patches fix lower-bound gaps and OOB reads in DFS referral
parsing, EA list traversal, posix SID bounds, change-notify offset,
snapshot enumeration, and SMB1 reparse point validation.

Note on overlap with a concurrent series: Zihan Xi's
[PATCH v2 0/2] "smb: client: fix create context out-of-bounds reads"
(Message-ID: <cover.1787486936.git.zihanx@nebusec.ai>) touches
smb2_parse_contexts() and parse_posix_ctxt() independently.  Patch 11
here addresses the same function (smb2_parse_contexts()) but focuses on
complementary issues that their series does not cover: NameOffset
validation (lower and upper bounds) and gating all three handler
dispatches on a non-zero DataLength to prevent zero-DataLength contexts
from exercising parse_lease_buf, parse_query_id_ctxt, or parse_posix_ctxt.
Their per-context cc_len bounding and lease/QFid minimum-length checks
are not duplicated here.  parse_posix_ctxt() DataLength validation is
omitted from this series entirely since their patch 2/2 addresses it.

Frank Sorenson (11):
  smb: client: fix NextCommand bounds and aliasing UAF in
    receive_encrypted_standard()
  smb: client: validate PDU length before smb2_get_data_area_len()
    struct access
  smb: client: fix server->total_read not tracking sub-PDU size in
    receive_encrypted_standard()
  smb: client: fix missing lower-bound check on DFS referral string
    offsets
  smb: client: fix missing lower-bound on Next field in
    parse_server_interfaces()
  smb: client: fix OOB struct field reads in move_smb2_ea_to_cifs()
  smb: client: fix missing iov bounds check in parse_posix_sids()
  smb: client: fix underflow in is_valid_oplock_break() notify offset
    check
  smb: client: fix potential OOB read in smb3_enum_snapshots()
  smb: client: fix incomplete bounds check on reparse buffer in
    cifs_query_reparse_point()
  smb: client: fix NameOffset and Next field validation in
    smb2_parse_contexts()

 fs/smb/client/cifssmb.c   |  2 +-
 fs/smb/client/misc.c      | 12 +++++++--
 fs/smb/client/smb1misc.c  |  3 ++-
 fs/smb/client/smb2inode.c | 11 ++++++++
 fs/smb/client/smb2misc.c  | 53 +++++++++++++++++++++++++++++++++++++++
 fs/smb/client/smb2ops.c   | 45 +++++++++++++++++++++++----------
 fs/smb/client/smb2pdu.c   | 11 ++++----
 fs/smb/client/trace.h     |  1 +
 8 files changed, 116 insertions(+), 22 deletions(-)

-- 
2.55.0

[PATCH 01/11] smb: client: fix NextCommand bounds and aliasing UAF in receive_encrypted_standard()

[Frank Sorenson]

Four related bugs in receive_encrypted_standard():

Lower bound: next_cmd is not checked against MID_HEADER_SIZE(server).  A
non-zero NextCommand smaller than the SMB2 header size passes the
upper-bound check; memcpy pulls bytes from within the current PDU header
into next_buffer, and the next iteration casts that region as a fresh
smb2_hdr.

Upper bound / trailing slice: the original check uses strict greater-than
(`next_cmd > pdu_length`), so next_cmd == pdu_length passes; memcpy copies
zero bytes, smb2_check_message() rejects the zeroed buffer, ret != 0, and
free_rsp_buf() releases next_buffer (aliasing server->bigbuf) while
allocate_buffers() still holds it: write-after-free.  Even with strict
less-than enforced, a next_cmd satisfying next_cmd < pdu_length but
pdu_length - next_cmd < MID_HEADER_SIZE(server) leaves a trailing slice too
small to hold an SMB2 header.  The next iteration reads shdr->NextCommand
from uninitialized slab content; if it reads as 0, the else-if (ret != 0)
branch frees next_buffer, again aliasing server->bigbuf: another
write-after-free.  Replace with three unsigned comparisons that avoid
addition: `next_cmd > pdu_length` to guard against underflow, then
`pdu_length - next_cmd < MID_HEADER_SIZE(server)` for the trailing slice
minimum.  The addition form (`(size_t)next_cmd +
MID_HEADER_SIZE(server) > pdu_length`) wraps to zero on 32-bit
kernels for next_cmd near UINT_MAX, silently admitting out-of-bounds.

Aliasing after goto: once server->bigbuf = buf = next_buffer and the goto
fires, next_buffer still holds the live server->bigbuf pointer.  If
smb2_check_message() fails on that iteration, ret != 0 and the else-if path
frees next_buffer, freeing server->bigbuf while it remains referenced.
Set next_buffer = NULL before goto one_more to prevent the aliased pointer
from being freed as a standalone buffer.

Decrypted extent: decrypt_raw_data() moves plaintext to buf[0..buf_size-1]
via memmove, where buf_size = pdu_length -
sizeof(struct smb2_transform_hdr).  Using pdu_length as the upper bound
allows next_cmd values in (buf_size, pdu_length], letting buf + next_cmd
land in the stale 52-byte transform-header residue and memcpy to copy
stale ciphertext into next_buffer as if it were the next SMB2 frame.
Assign pdu_length = buf_size after successful decryption so bounds checks
and the per-iteration decrement operate on the correct plaintext extent.

smb2_next_header() already enforces the equivalent lower-bound:

    if (unlikely(*noff && *noff < MID_HEADER_SIZE(server)))
        return -EINVAL;

Apply the same constraint here.

Fixes: b24df3e30cbf ("cifs: update receive_encrypted_standard to handle compounded responses")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/smb2ops.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 7d6738ffcb80..e75420dbe950 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -5242,6 +5242,7 @@ receive_encrypted_standard(struct TCP_Server_Info *server,
 	length = decrypt_raw_data(server, buf, buf_size, NULL, false);
 	if (length)
 		return length;
+	pdu_length = buf_size;
 
 	next_is_large = server->large_buf;
 one_more:
@@ -5254,8 +5255,15 @@ receive_encrypted_standard(struct TCP_Server_Info *server,
 	}
 
 	if (next_cmd) {
-		if (WARN_ON_ONCE(next_cmd > pdu_length))
+		if (next_cmd < MID_HEADER_SIZE(server) ||
+		    next_cmd > pdu_length ||
+		    pdu_length - next_cmd < MID_HEADER_SIZE(server)) {
+			unsigned int max_next = pdu_length > (unsigned int)MID_HEADER_SIZE(server) ?
+					pdu_length - (unsigned int)MID_HEADER_SIZE(server) : 0;
+			cifs_server_dbg(VFS, "invalid NextCommand offset %u out of range [%zu, %u]\n",
+					next_cmd, MID_HEADER_SIZE(server), max_next);
 			return -1;
+		}
 		if (next_is_large)
 			next_buffer = (char *)cifs_buf_get();
 		else
@@ -5291,6 +5299,7 @@ receive_encrypted_standard(struct TCP_Server_Info *server,
 			server->bigbuf = buf = next_buffer;
 		else
 			server->smallbuf = buf = next_buffer;
+		next_buffer = NULL;
 		goto one_more;
 	} else if (ret != 0) {
 		/*
-- 
2.55.0

[PATCH 02/11] smb: client: validate PDU length before smb2_get_data_area_len() struct access

[Frank Sorenson]

smb2_check_message() validates StructureSize2 but not that the received
PDU is large enough to hold the fixed response struct before calling
__smb2_calc_size() -> smb2_get_data_area_len().

smb2_get_data_area_len() reads command-specific struct fields (e.g.,
CreateContextsOffset at offset 144 in smb2_create_rsp) to locate the
data area before the length is validated.  A rogue server can send a
truncated response that passes the StructureSize2 check but causes
smb2_get_data_area_len() to read stale kmalloc'd content.

Add smb2_min_pdu_len[], parallel to smb2_rsp_struct_sizes[], holding
the minimum PDU size (sizeof the fixed response struct) for each command
with a data area.  Reject responses shorter than this minimum before
calling __smb2_calc_size().

The guard condition is the complement of smb2_get_data_area_len()'s
early-return predicate: fires when Status == 0, Status ==
STATUS_MORE_PROCESSING_REQUIRED, or StructureSize2 !=
SMB2_ERROR_STRUCTURE_SIZE2_LE.

SESSION_SETUP with STATUS_MORE_PROCESSING_REQUIRED is covered by the
second term; READ and IOCTL with STATUS_BUFFER_OVERFLOW by the third —
their StructureSize2 (17 and 49 respectively) differs from the error
struct size 9, so smb2_get_data_area_len() reaches the switch without
the guard.

Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/smb2misc.c | 53 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/fs/smb/client/smb2misc.c b/fs/smb/client/smb2misc.c
index 9068175e57cd..7bf7c602cda0 100644
--- a/fs/smb/client/smb2misc.c
+++ b/fs/smb/client/smb2misc.c
@@ -85,6 +85,49 @@ static const __le16 smb2_rsp_struct_sizes[NUMBER_OF_SMB2_COMMANDS] = {
 	/* SMB2_OPLOCK_BREAK */ cpu_to_le16(24)
 };
 
+/*
+ * Minimum received PDU size for commands whose fixed response struct is read
+ * by smb2_get_data_area_len() before the packet length is validated.  Must
+ * be non-zero for every command where has_smb2_data_area[] is true; zero
+ * otherwise (guard in smb2_check_message() is skipped).  Keep in sync with
+ * has_smb2_data_area[] above: adding a data area for a currently-zero command
+ * requires a matching sizeof() entry here.
+ */
+static const size_t smb2_min_pdu_len[NUMBER_OF_SMB2_COMMANDS] = {
+	/* SMB2_NEGOTIATE */       sizeof(struct smb2_negotiate_rsp),
+	/* SMB2_SESSION_SETUP */   sizeof(struct smb2_sess_setup_rsp),
+	/* SMB2_LOGOFF */          0,
+	/* SMB2_TREE_CONNECT */    0,
+	/* SMB2_TREE_DISCONNECT */ 0,
+	/* SMB2_CREATE */          sizeof(struct smb2_create_rsp),
+	/* SMB2_CLOSE */           0,
+	/* SMB2_FLUSH */           0,
+	/* SMB2_READ */            sizeof(struct smb2_read_rsp),
+	/* SMB2_WRITE */           0,
+	/* SMB2_LOCK */            0,
+	/* SMB2_IOCTL */           sizeof(struct smb2_ioctl_rsp),
+	/* SMB2_CANCEL */          0,
+	/* SMB2_ECHO */            0,
+	/* SMB2_QUERY_DIRECTORY */ sizeof(struct smb2_query_directory_rsp),
+	/* SMB2_CHANGE_NOTIFY */   sizeof(struct smb2_change_notify_rsp),
+	/* SMB2_QUERY_INFO */      sizeof(struct smb2_query_info_rsp),
+	/* SMB2_SET_INFO */        0,
+	/* SMB2_OPLOCK_BREAK */    0,
+};
+
+/* Enforce: smb2_min_pdu_len[x] != 0 for every x where has_smb2_data_area[x]. */
+static void __maybe_unused smb2_check_min_pdu_len_table(void)
+{
+	BUILD_BUG_ON(!smb2_min_pdu_len[SMB2_NEGOTIATE_HE]);
+	BUILD_BUG_ON(!smb2_min_pdu_len[SMB2_SESSION_SETUP_HE]);
+	BUILD_BUG_ON(!smb2_min_pdu_len[SMB2_CREATE_HE]);
+	BUILD_BUG_ON(!smb2_min_pdu_len[SMB2_READ_HE]);
+	BUILD_BUG_ON(!smb2_min_pdu_len[SMB2_IOCTL_HE]);
+	BUILD_BUG_ON(!smb2_min_pdu_len[SMB2_QUERY_DIRECTORY_HE]);
+	BUILD_BUG_ON(!smb2_min_pdu_len[SMB2_CHANGE_NOTIFY_HE]);
+	BUILD_BUG_ON(!smb2_min_pdu_len[SMB2_QUERY_INFO_HE]);
+}
+
 #define SMB311_NEGPROT_BASE_SIZE (sizeof(struct smb2_hdr) + sizeof(struct smb2_negotiate_rsp))
 
 static __u32 get_neg_ctxt_len(struct smb2_hdr *hdr, __u32 len,
@@ -233,6 +276,16 @@ smb2_check_message(char *buf, unsigned int pdu_len, unsigned int len,
 		}
 	}
 
+	if ((shdr->Status == 0 ||
+	     shdr->Status == STATUS_MORE_PROCESSING_REQUIRED ||
+	     pdu->StructureSize2 != SMB2_ERROR_STRUCTURE_SIZE2_LE) &&
+	    smb2_min_pdu_len[command] &&
+	    len < smb2_min_pdu_len[command]) {
+		cifs_dbg(VFS, "SMB2 command %d response too short: %u < %zu\n",
+			 command, len, smb2_min_pdu_len[command]);
+		return 1;
+	}
+
 	have_data = false;
 	data_area_overlap = false;
 	calc_len = __smb2_calc_size(buf, &have_data, &data_area_overlap);
-- 
2.55.0

[PATCH 03/11] smb: client: fix server->total_read not tracking sub-PDU size in receive_encrypted_standard()

[Frank Sorenson]

receive_encrypted_standard() processes compound encrypted frames by
iterating sub-PDUs, decrementing pdu_length by NextCommand on each
hop but leaving server->total_read at the original full-frame size.
cifs_handle_standard() passes server->total_read as len to
smb2_check_message(), so every length guard in smb2_check_message()
(smb2_rsp_struct_sizes[], smb2_min_pdu_len[]) operates on the
full-frame size rather than the current sub-PDU.

A malicious server can craft a compound encrypted frame where any
non-last sub-PDU is shorter than the fixed response struct it
declares.  The oversized len bypasses both guards, smb2_get_data_area_len()
reads struct fields at offsets past the received data, and the
data-area length arithmetic produces a spurious pointer into
uninitialized slab content or the next sub-PDU, which a rogue server
controls.

At the top of each iteration, pdu_length is the full remaining
compound tail (current sub-PDU + all subsequent ones), not the current
sub-PDU alone.  For a non-last sub-PDU, server->total_read must be
derived from next_cmd, not pdu_length.  Read next_cmd before assigning
server->total_read, then use `next_cmd ? next_cmd : pdu_length` so
that smb2_check_message() sees the actual per-sub-PDU extent on every
pass through the loop.

Fixes: b24df3e30cbf ("cifs: update receive_encrypted_standard to handle compounded responses")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/smb2ops.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index e75420dbe950..1bc3f8266eda 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -5248,6 +5248,7 @@ receive_encrypted_standard(struct TCP_Server_Info *server,
 one_more:
 	shdr = (struct smb2_hdr *)buf;
 	next_cmd = le32_to_cpu(shdr->NextCommand);
+	server->total_read = next_cmd ? next_cmd : pdu_length;
 
 	if (*num_mids >= MAX_COMPOUND) {
 		cifs_server_dbg(VFS, "too many PDUs in compound\n");
-- 
2.55.0

[PATCH 04/11] smb: client: fix missing lower-bound check on DFS referral string offsets

[Frank Sorenson]

parse_dfs_referrals() checks DfsPathOffset and NetworkAddressOffset
against the buffer end but not against sizeof(*ref).  An offset smaller
than sizeof(struct dfs_referral_level_3) places the derived pointer
inside the referral header, causing cifs_strndup_from_utf16() to read
struct fields as string data.

Require each offset to be at least sizeof(*ref).

Fixes: 4ecce920e13a ("CIFS: move DFS response parsing out of SMB1 code")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/misc.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
index 46e1382e8e04..bc7cccebf387 100644
--- a/fs/smb/client/misc.c
+++ b/fs/smb/client/misc.c
@@ -787,7 +787,11 @@ parse_dfs_referrals(struct get_dfs_referral_rsp *rsp, u32 rsp_size,
 		node->ref_flag = le16_to_cpu(ref->ReferralEntryFlags);
 
 		/* copy DfsPath */
-		if (le16_to_cpu(ref->DfsPathOffset) > data_end - (char *)ref) {
+		if (le16_to_cpu(ref->DfsPathOffset) < sizeof(*ref) ||
+		    le16_to_cpu(ref->DfsPathOffset) > data_end - (char *)ref) {
+			cifs_dbg(VFS, "%s: DfsPathOffset %u out of range [%zu, %td]\n",
+				 __func__, le16_to_cpu(ref->DfsPathOffset),
+				 sizeof(*ref), data_end - (char *)ref);
 			rc = -EINVAL;
 			goto parse_DFS_referrals_exit;
 		}
@@ -801,7 +805,11 @@ parse_dfs_referrals(struct get_dfs_referral_rsp *rsp, u32 rsp_size,
 		}
 
 		/* copy link target UNC */
-		if (le16_to_cpu(ref->NetworkAddressOffset) > data_end - (char *)ref) {
+		if (le16_to_cpu(ref->NetworkAddressOffset) < sizeof(*ref) ||
+		    le16_to_cpu(ref->NetworkAddressOffset) > data_end - (char *)ref) {
+			cifs_dbg(VFS, "%s: NetworkAddressOffset %u out of range [%zu, %td]\n",
+				 __func__, le16_to_cpu(ref->NetworkAddressOffset),
+				 sizeof(*ref), data_end - (char *)ref);
 			rc = -EINVAL;
 			goto parse_DFS_referrals_exit;
 		}
-- 
2.55.0

[PATCH 05/11] smb: client: fix missing lower-bound on Next field in parse_server_interfaces()

[Frank Sorenson]

parse_server_interfaces() validates the server-supplied Next offset
against bytes_left but not against sizeof(*p).  A small non-zero value
passes the upper-bound check yet advances p by less than one struct,
causing the next iteration to read misaligned, overlapping fields.

Add next < sizeof(*p) to the existing bounds check.

Fixes: 7d34ec36abb8 ("smb3: fix for slab out of bounds on mount to ksmbd")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/smb2ops.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 1bc3f8266eda..9065c94e5ec6 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -785,9 +785,9 @@ parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf,
 			break;
 		}
 		/* Validate that Next doesn't point beyond the buffer */
-		if (next > bytes_left) {
-			cifs_dbg(VFS, "%s: invalid Next pointer %zu > %zd\n",
-				 __func__, next, bytes_left);
+		if (next < sizeof(*p) || next > bytes_left) {
+			cifs_dbg(VFS, "%s: invalid Next pointer %zu out of range [%zu, %zd]\n",
+				 __func__, next, sizeof(*p), bytes_left);
 			rc = -EINVAL;
 			goto out;
 		}
-- 
2.55.0

[PATCH 06/11] smb: client: fix OOB struct field reads in move_smb2_ea_to_cifs()

[Frank Sorenson]

The while (src_size > 0) loop guard allows iteration after
next_entry_offset advances src past the point where a full struct fits
in src_size.  Reads of ea_name_length and ea_value_length on the next
iteration are then out-of-bounds.

Require src_size >= sizeof(*src) before reading any struct field, and
reject next_entry_offset values smaller than sizeof(*src).

For calls where the server returns an EA list with an invalid
next_entry_offset, the error returned to userspace changes from
-ENODATA (getxattr) or -ERANGE (listxattr) to -EIO, correctly
signalling a server protocol error rather than "attribute not present"
or "output buffer too small".

Fixes: 95907fea4fd8 ("cifs: Add support for reading attributes on SMB2+")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/smb2ops.c | 19 +++++++++++--------
 fs/smb/client/trace.h   |  1 +
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 9065c94e5ec6..37c1144f3d57 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -1053,8 +1053,9 @@ move_smb2_ea_to_cifs(char *dst, size_t dst_size,
 	char *name, *value;
 	size_t buf_size = dst_size;
 	size_t name_len, value_len, user_name_len;
+	u32 next_off;
 
-	while (src_size > 0) {
+	while (src_size >= sizeof(*src)) {
 		name_len = (size_t)src->ea_name_length;
 		value_len = (size_t)le16_to_cpu(src->ea_value_length);
 
@@ -1110,14 +1111,16 @@ move_smb2_ea_to_cifs(char *dst, size_t dst_size,
 		if (!src->next_entry_offset)
 			break;
 
-		if (src_size < le32_to_cpu(src->next_entry_offset)) {
-			/* stop before overrun buffer */
-			rc = -ERANGE;
-			break;
+		next_off = le32_to_cpu(src->next_entry_offset);
+		if (next_off < sizeof(*src) || src_size < next_off) {
+			cifs_dbg(FYI, "EA next_entry_offset %u out of range [%zu, %zu]\n",
+				 next_off, sizeof(*src), src_size);
+			rc = smb_EIO2(smb_eio_trace_ea_next_offset,
+				      next_off, src_size);
+			goto out;
 		}
-		src_size -= le32_to_cpu(src->next_entry_offset);
-		src = (void *)((char *)src +
-			       le32_to_cpu(src->next_entry_offset));
+		src_size -= next_off;
+		src = (void *)((char *)src + next_off);
 	}
 
 	/* didn't find the named attribute */
diff --git a/fs/smb/client/trace.h b/fs/smb/client/trace.h
index 12241abb8e2e..6da395abae14 100644
--- a/fs/smb/client/trace.h
+++ b/fs/smb/client/trace.h
@@ -27,6 +27,7 @@
 	EM(smb_eio_trace_copychunk_overcopy_c,		"copychunk_overcopy_c") \
 	EM(smb_eio_trace_create_rsp_too_small,		"create_rsp_too_small") \
 	EM(smb_eio_trace_dfsref_no_rsp,			"dfsref_no_rsp") \
+	EM(smb_eio_trace_ea_next_offset,		"ea_next_offset") \
 	EM(smb_eio_trace_ea_overrun,			"ea_overrun") \
 	EM(smb_eio_trace_extract_will_pin,		"extract_will_pin") \
 	EM(smb_eio_trace_forced_shutdown,		"forced_shutdown") \
-- 
2.55.0

[PATCH 07/11] smb: client: fix missing iov bounds check in parse_posix_sids()

[Frank Sorenson]

parse_posix_sids() derives sidsbuf_end from the server-supplied out_len
without validating against the received iov:

    sidsbuf_end = sidsbuf + out_len - qi_len;

An inflated out_len places sidsbuf_end past the iov, defeating the
bounds guards in posix_info_sid_size() and allowing reads into adjacent
kernel memory.

Reject responses where out_len places sidsbuf_end past the received iov
or causes pointer wraparound.

Fixes: a90f37e3d7ac ("smb: client: parse owner/group when creating reparse points")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/smb2inode.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 98ea5c6c34af..c7a2dddb3959 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -77,6 +77,17 @@ static int parse_posix_sids(struct cifs_open_info_data *data,
 
 	sidsbuf = (u8 *)qi + le16_to_cpu(qi->OutputBufferOffset) + qi_len;
 	sidsbuf_end = sidsbuf + out_len - qi_len;
+	if (sidsbuf_end < sidsbuf) {
+		cifs_dbg(VFS, "%s: server-supplied out_len %u caused pointer wraparound\n",
+			 __func__, out_len);
+		return -EINVAL;
+	}
+	if (sidsbuf_end > (u8 *)rsp_iov->iov_base + rsp_iov->iov_len) {
+		cifs_dbg(VFS, "%s: server-supplied out_len %u overruns iov by %td bytes\n",
+			 __func__, out_len,
+			 sidsbuf_end - ((u8 *)rsp_iov->iov_base + rsp_iov->iov_len));
+		return -EINVAL;
+	}
 
 	owner_len = posix_info_sid_size(sidsbuf, sidsbuf_end);
 	if (owner_len == -1)
-- 
2.55.0

[PATCH 08/11] smb: client: fix underflow in is_valid_oplock_break() notify offset check

[Frank Sorenson]

The check intended to bound data_offset against the received frame:

    if (data_offset > len - sizeof(struct file_notify_information))

underflows when len < sizeof(struct file_notify_information): the
subtraction wraps to a large value, the condition is false, and pnotify
is constructed from an unchecked data_offset.

Check len < sizeof(struct file_notify_information) first.

Fixes: 097f5863b1a0 ("cifs: read overflow in is_valid_oplock_break()")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/smb1misc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/smb1misc.c b/fs/smb/client/smb1misc.c
index cdfbbff24b72..a39c9140ceca 100644
--- a/fs/smb/client/smb1misc.c
+++ b/fs/smb/client/smb1misc.c
@@ -86,7 +86,8 @@ is_valid_oplock_break(char *buffer, struct TCP_Server_Info *srv)
 		if (get_bcc(buf) > sizeof(struct file_notify_information)) {
 			data_offset = le32_to_cpu(pSMBr->DataOffset);
 
-			if (data_offset >
+			if (len < sizeof(struct file_notify_information) ||
+			    data_offset >
 			    len - sizeof(struct file_notify_information)) {
 				cifs_dbg(FYI, "Invalid data_offset %u\n",
 					 data_offset);
-- 
2.55.0

[PATCH 09/11] smb: client: fix potential OOB read in smb3_enum_snapshots()

[Frank Sorenson]

When snapshot_array_size < GMT_TOKEN_SIZE, smb3_enum_snapshots()
unconditionally sets ret_data_len = sizeof(struct smb_snapshot_array),
silently inflating it beyond the actual server response size if the
server returned fewer bytes.  The subsequent copy_to_user() then reads
past the end of retbuf.

Reject responses shorter than sizeof(struct smb_snapshot_array) with -EIO.

Fixes: dcbf1c8af18a ("smb3: add ioctl to get a list of snapshots for volume or share")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/smb2ops.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 37c1144f3d57..77ff285ce1b6 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -2376,8 +2376,14 @@ smb3_enum_snapshots(const unsigned int xid, struct cifs_tcon *tcon,
 		 * and retry the ioctl again with larger array size sufficient
 		 * to hold all of the snapshot GMT tokens on the second try.
 		 */
-		if (snapshot_in.snapshot_array_size < GMT_TOKEN_SIZE)
+		if (snapshot_in.snapshot_array_size < GMT_TOKEN_SIZE) {
+			if (ret_data_len < sizeof(struct smb_snapshot_array)) {
+				rc = -EIO;
+				kfree(retbuf);
+				return rc;
+			}
 			ret_data_len = sizeof(struct smb_snapshot_array);
+		}
 
 		/*
 		 * We return struct SRV_SNAPSHOT_ARRAY, followed by
-- 
2.55.0

[PATCH 10/11] smb: client: fix incomplete bounds check on reparse buffer in cifs_query_reparse_point()

[Frank Sorenson]

The start >= end check before casting to struct reparse_data_buffer *
ensures the pointer is in range but not that end - start >= sizeof(*buf).
A server-supplied DataOffset leaving fewer than 8 bytes passes the check
but allows OOB reads of ReparseTag and ReparseDataLength.

Fold the minimum-size check into the existing condition using `end`.

Fixes: c13b779d26b3 ("cifs: Fix validation of SMB1 query reparse point response")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/cifssmb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
index f5aad5f61dce..acb533383a3e 100644
--- a/fs/smb/client/cifssmb.c
+++ b/fs/smb/client/cifssmb.c
@@ -3064,7 +3064,7 @@ int cifs_query_reparse_point(const unsigned int xid,
 
 	end = 2 + get_bcc(&io_rsp->hdr) + (__u8 *)&io_rsp->ByteCount;
 	start = (__u8 *)&io_rsp->hdr.Protocol + data_offset;
-	if (start >= end) {
+	if (start >= end || (size_t)(end - start) < sizeof(*buf)) {
 		rc = smb_EIO2(smb_eio_trace_qreparse_data_area,
 			      (unsigned long)start - (unsigned long)io_rsp,
 			      (unsigned long)end - (unsigned long)io_rsp);
-- 
2.55.0

[PATCH 11/11] smb: client: fix NameOffset and Next field validation in smb2_parse_contexts()

[Frank Sorenson]

Three related bounds issues in smb2_parse_contexts():

NameOffset: the existing 'noff + nlen > doff' check doesn't prevent
the name from starting inside the context header or extending past rem.
Replace it with noff >= sizeof(*cc) and noff + nlen <= rem, keeping
the name-before-data guard only when DataLength is non-zero.

Next: a non-zero Next smaller than sizeof(*cc) passes the existing
upper-bound check but creates an overlapping context pointer.  Add
off < sizeof(*cc) to reject it.

Dispatch: when DataLength is zero, calling a handler that reads the
data area (parse_lease_buf, parse_query_id_ctxt, parse_posix_ctxt)
reads stale memory past the declared context.  A rogue server can
craft a zero-DataLength context whose name matches a known tag (e.g.
'RqLs') and bypass the name-vs-data-offset guard, causing
parse_lease_buf to record a lease key and state from uninitialized
kmalloc content.  Gate all three dispatch paths on dlen > 0.

Fixes: af1689a9b770 ("smb: client: fix potential OOBs in smb2_parse_contexts()")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
 fs/smb/client/smb2pdu.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index dea05aeb53a1..dda7a1a88b44 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -2459,22 +2459,23 @@ int smb2_parse_contexts(struct TCP_Server_Info *server,
 
 		noff = le16_to_cpu(cc->NameOffset);
 		nlen = le16_to_cpu(cc->NameLength);
-		if (noff + nlen > doff)
+		if (noff < sizeof(*cc) || noff + nlen > rem ||
+		    (dlen && noff + nlen > 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:
@@ -2488,7 +2489,7 @@ int smb2_parse_contexts(struct TCP_Server_Info *server,
 		off = le32_to_cpu(cc->Next);
 		if (!off)
 			break;
-		if (check_sub_overflow(rem, off, &rem))
+		if (off < sizeof(*cc) || check_sub_overflow(rem, off, &rem))
 			return -EINVAL;
 		cc = (struct create_context *)((u8 *)cc + off);
 	}
-- 
2.55.0

[PATCH v2] smb: client: fix OOB struct field reads in move_smb2_ea_to_cifs()

[Frank Sorenson]

The while (src_size > 0) loop guard allows iteration after
next_entry_offset advances src past the point where a full struct fits
in src_size.  Reads of ea_name_length and ea_value_length on the next
iteration are then out-of-bounds.

Require src_size >= sizeof(*src) before reading any struct field,
and reject next_entry_offset values that are smaller than
sizeof(*src) or that leave fewer than sizeof(*src) bytes remaining
after advancing.

For calls where the server returns an EA list with an invalid
next_entry_offset, the error returned to userspace changes from
-ENODATA (getxattr) or -ERANGE (listxattr) to -EIO, correctly
signalling a server protocol error rather than "attribute not present"
or "output buffer too small".

Fixes: 95907fea4fd8 ("cifs: Add support for reading attributes on SMB2+")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
---
v2 changes:
 -  reject next_entry_offset values that leave fewer than
    sizeof(*src) bytes remainint after advancing.

 fs/smb/client/smb2ops.c | 25 +++++++++++++++++--------
 fs/smb/client/trace.h   |  1 +
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 4dd9dd55ab4d..c866d7c8c7dd 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -1053,8 +1053,9 @@ move_smb2_ea_to_cifs(char *dst, size_t dst_size,
 	char *name, *value;
 	size_t buf_size = dst_size;
 	size_t name_len, value_len, user_name_len;
+	u32 next_off;
 
-	while (src_size > 0) {
+	while (src_size >= sizeof(*src)) {
 		name_len = (size_t)src->ea_name_length;
 		value_len = (size_t)le16_to_cpu(src->ea_value_length);
 
@@ -1110,14 +1111,22 @@ move_smb2_ea_to_cifs(char *dst, size_t dst_size,
 		if (!src->next_entry_offset)
 			break;
 
-		if (src_size < le32_to_cpu(src->next_entry_offset)) {
-			/* stop before overrun buffer */
-			rc = -ERANGE;
-			break;
+		next_off = le32_to_cpu(src->next_entry_offset);
+		if (next_off < sizeof(*src) || src_size < next_off) {
+			cifs_dbg(FYI, "EA next_entry_offset %u out of range [%zu, %zu]\n",
+				 next_off, sizeof(*src), src_size);
+			rc = smb_EIO2(smb_eio_trace_ea_next_offset,
+				      next_off, src_size);
+			goto out;
+		}
+		src_size -= next_off;
+		src = (void *)((char *)src + next_off);
+		if (src_size > 0 && src_size < sizeof(*src)) {
+			cifs_dbg(FYI, "EA next_entry_offset %u left truncated entry (%zu bytes)\n",
+				 next_off, src_size);
+			rc = smb_EIO2(smb_eio_trace_ea_next_offset, next_off, src_size);
+			goto out;
 		}
-		src_size -= le32_to_cpu(src->next_entry_offset);
-		src = (void *)((char *)src +
-			       le32_to_cpu(src->next_entry_offset));
 	}
 
 	/* didn't find the named attribute */
diff --git a/fs/smb/client/trace.h b/fs/smb/client/trace.h
index 12241abb8e2e..6da395abae14 100644
--- a/fs/smb/client/trace.h
+++ b/fs/smb/client/trace.h
@@ -27,6 +27,7 @@
 	EM(smb_eio_trace_copychunk_overcopy_c,		"copychunk_overcopy_c") \
 	EM(smb_eio_trace_create_rsp_too_small,		"create_rsp_too_small") \
 	EM(smb_eio_trace_dfsref_no_rsp,			"dfsref_no_rsp") \
+	EM(smb_eio_trace_ea_next_offset,		"ea_next_offset") \
 	EM(smb_eio_trace_ea_overrun,			"ea_overrun") \
 	EM(smb_eio_trace_extract_will_pin,		"extract_will_pin") \
 	EM(smb_eio_trace_forced_shutdown,		"forced_shutdown") \
-- 
2.55.0

Re: [PATCH 00/11] smb: client: fix OOB reads and UAFs in SMB2/3 receive paths

[Yunseong Kim]

Hi Frank,

Thank you, for your security works!

On Sun, 23 Aug 2026 13:57:56 -0500 Frank Sorenson <sorenson@redhat.com> wrote:

> This series fixes eleven bounds-checking defects in the SMB2/3 client,
> all reachable from a malicious or compromised server.
> 
> Patches 1-3 address the compound encrypted frame processing path:
> 
> Patch 1 fixes four interacting bugs in receive_encrypted_standard():
> a missing lower bound on NextCommand, an off-by-one upper bound that
> admitted trailing slices too small for an SMB2 header (producing a
> write-after-free via next_buffer aliasing server->bigbuf), a stale
> next_buffer pointer not cleared before goto one_more, and use of the
> pre-decryption pdu_length instead of the plaintext extent for bounds
> checking.
> 
> Patch 2 adds smb2_min_pdu_len[], a per-command table of minimum response
> struct sizes, and uses it in smb2_check_message() to reject responses
> too short for smb2_get_data_area_len() to safely read command-specific
> struct fields.
> 
> Patch 3 fixes server->total_read tracking in receive_encrypted_standard()
> so that smb2_check_message() sees the actual per-sub-PDU size rather than
> the full remaining compound tail.  Without this, a rogue server can craft
> a compound frame where any non-last sub-PDU is shorter than its declared
> fixed struct, bypassing the guards added in patch 2.
> 
> The remaining patches fix lower-bound gaps and OOB reads in DFS referral
> parsing, EA list traversal, posix SID bounds, change-notify offset,
> snapshot enumeration, and SMB1 reparse point validation.
> 
> Note on overlap with a concurrent series: Zihan Xi's
> [PATCH v2 0/2] "smb: client: fix create context out-of-bounds reads"
> (Message-ID: <cover.1787486936.git.zihanx@nebusec.ai>) touches
> smb2_parse_contexts() and parse_posix_ctxt() independently.  Patch 11
> here addresses the same function (smb2_parse_contexts()) but focuses on
> complementary issues that their series does not cover: NameOffset
> validation (lower and upper bounds) and gating all three handler
> dispatches on a non-zero DataLength to prevent zero-DataLength contexts
> from exercising parse_lease_buf, parse_query_id_ctxt, or parse_posix_ctxt.
> Their per-context cc_len bounding and lease/QFid minimum-length checks
> are not duplicated here.  parse_posix_ctxt() DataLength validation is
> omitted from this series entirely since their patch 2/2 addresses it.
> 
> Frank Sorenson (11):
>   smb: client: fix NextCommand bounds and aliasing UAF in
>     receive_encrypted_standard()
>   smb: client: validate PDU length before smb2_get_data_area_len()
>     struct access
>   smb: client: fix server->total_read not tracking sub-PDU size in
>     receive_encrypted_standard()
>   smb: client: fix missing lower-bound check on DFS referral string
>     offsets
>   smb: client: fix missing lower-bound on Next field in
>     parse_server_interfaces()
>   smb: client: fix OOB struct field reads in move_smb2_ea_to_cifs()
>   smb: client: fix missing iov bounds check in parse_posix_sids()
>   smb: client: fix underflow in is_valid_oplock_break() notify offset
>     check
>   smb: client: fix potential OOB read in smb3_enum_snapshots()
>   smb: client: fix incomplete bounds check on reparse buffer in
>     cifs_query_reparse_point()
>   smb: client: fix NameOffset and Next field validation in
>     smb2_parse_contexts()
> 
>  fs/smb/client/cifssmb.c   |  2 +-
>  fs/smb/client/misc.c      | 12 +++++++--
>  fs/smb/client/smb1misc.c  |  3 ++-
>  fs/smb/client/smb2inode.c | 11 ++++++++
>  fs/smb/client/smb2misc.c  | 53 +++++++++++++++++++++++++++++++++++++++
>  fs/smb/client/smb2ops.c   | 45 +++++++++++++++++++++++----------
>  fs/smb/client/smb2pdu.c   | 11 ++++----
>  fs/smb/client/trace.h     |  1 +
>  8 files changed, 116 insertions(+), 22 deletions(-)
> 
> -- 
> 2.55.0
> 
> 

Just a small question: I wasn't able to verify the call stack from this
patch series alone. Is there a reproducible test case or script that triggers
the issue?

As a security researcher, I'd also like to independently verify the findings
and cross-check the behavior on my side. Any reproducer or additional details
would be greatly appreciated.

The reason I'm asking is that I've been working on CI coverage for SMB. I hope
we can integrate tests for this issue as well, so that similar regressions can
be detected and prevented in the future.

Best regards,
Yunseong

Re: [PATCH v2] smb: client: fix OOB struct field reads in move_smb2_ea_to_cifs()

[Namjae Jeon]

On Wed, Aug 26, 2026 at 7:13 AM Frank Sorenson <sorenson@redhat.com> wrote:
>
> The while (src_size > 0) loop guard allows iteration after
> next_entry_offset advances src past the point where a full struct fits
> in src_size.  Reads of ea_name_length and ea_value_length on the next
> iteration are then out-of-bounds.
>
> Require src_size >= sizeof(*src) before reading any struct field,
> and reject next_entry_offset values that are smaller than
> sizeof(*src) or that leave fewer than sizeof(*src) bytes remaining
> after advancing.
>
> For calls where the server returns an EA list with an invalid
> next_entry_offset, the error returned to userspace changes from
> -ENODATA (getxattr) or -ERANGE (listxattr) to -EIO, correctly
> signalling a server protocol error rather than "attribute not present"
> or "output buffer too small".
>
> Fixes: 95907fea4fd8 ("cifs: Add support for reading attributes on SMB2+")
> Cc: stable@vger.kernel.org
> Signed-off-by: Frank Sorenson <sorenson@redhat.com>
> ---
> v2 changes:
>  -  reject next_entry_offset values that leave fewer than
>     sizeof(*src) bytes remainint after advancing.
Since maintainers might accidentally pick up the v1 patch, I think it
would be better to resend the entire patch-set as v2.

Re: [PATCH 00/11] smb: client: fix OOB reads and UAFs in SMB2/3 receive paths

[Frank Sorenson]

On 8/25/26 6:54 PM, Yunseong Kim wrote:
> Hi Frank,
>
> Thank you, for your security works!
>
> On Sun, 23 Aug 2026 13:57:56 -0500 Frank Sorenson <sorenson@redhat.com> wrote:
...
> Just a small question: I wasn't able to verify the call stack from this
> patch series alone. Is there a reproducible test case or script that triggers
> the issue?
Not for most of them, although I'm sure I could come up with something.
> As a security researcher, I'd also like to independently verify the findings
> and cross-check the behavior on my side. Any reproducer or additional details
> would be greatly appreciated.
>
> The reason I'm asking is that I've been working on CI coverage for SMB. I hope
> we can integrate tests for this issue as well, so that similar regressions can
> be detected and prevented in the future.

I ran a static analysis tool on fs/smb/client looking for potential 
bounds issues, had Claude filter out false positives, then started 
looking at the findings for what could be fixed.

The bounds checker is at https://github.com/fsorenson/kernel_tools

# python3 bounds_checker/cli.py --kernel-source /home/src/linux 
--source-dir fs/smb/client

(add --llm to run the static analysis findings through an llm, but it'll 
really chew up your usage...  should work with Gemini & Claude, which 
it'll figure out from environment variables GOOGLE_CLOUD_PROJECT (Gemini 
via Vertex AI), GEMINI_API_KEY, ANTHROPIC_VERTEX_PROJECT_ID, 
ANTHROPIC_API_KEY)


Frank

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