Linux CIFS filesystem development
 help / color / mirror / Atom feed
From: Frank Sorenson <sorenson@redhat.com>
To: linux-cifs@vger.kernel.org, pc@manguebit.org
Cc: linkinjeon@kernel.org, ronniesahlberg@gmail.com,
	sprasad@microsoft.com, tom@talpey.com, bharathsm@microsoft.com
Subject: [PATCH v5 2/9] smb: client: validate minimum PDU size before smb2_get_data_area_len()
Date: Wed, 16 Sep 2026 16:33:53 -0500	[thread overview]
Message-ID: <20260916213406.1496960-3-sorenson@redhat.com> (raw)
In-Reply-To: <20260916213406.1496960-1-sorenson@redhat.com>

__smb2_calc_size() calls smb2_get_data_area_len(), which reads
command-specific struct fields to locate the data area. However,
smb2_check_message() only validates StructureSize2, meaning a truncated
response could cause smb2_get_data_area_len() to read out-of-bounds.

Replace has_smb2_data_area[] with smb2_min_pdu_len[], which is now
used to indicate both whether a command's response has a data area
and the size of that fixed response struct.  A non-zero entry means
the command has a data area, and is the minimum length required
before the struct is read.

For each command with a data area, PDUs shorter than this minimum size
are rejected instead of parsed.

The minimum is not applied to SMB2 error responses, which carry only
the 9-byte error body, the same exemption the StructureSize2 check
above it already makes.  STATUS_MORE_PROCESSING_REQUIRED is
treated as a normal reply, since an in-progress SESSION_SETUP
response carries a full body and a security blob.

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

diff --git a/fs/smb/client/smb2misc.c b/fs/smb/client/smb2misc.c
index 9068175e57cd..0cfe60ae42c3 100644
--- a/fs/smb/client/smb2misc.c
+++ b/fs/smb/client/smb2misc.c
@@ -85,6 +85,36 @@ 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 response carries a
+ * variable-length data area.  A non-zero entry marks the command as
+ * having one, and gives the length smb2_check_message() requires
+ * before smb2_get_data_area_len() reads the offset and length fields
+ * out of the fixed response struct.
+ */
+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,
+};
+
+#define smb2_has_data_area(cmd) (smb2_min_pdu_len[cmd] != 0)
 #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 +263,16 @@ smb2_check_message(char *buf, unsigned int pdu_len, unsigned int len,
 		}
 	}
 
+	if ((shdr->Status == STATUS_SUCCESS ||
+	     shdr->Status == STATUS_MORE_PROCESSING_REQUIRED ||
+	     pdu->StructureSize2 != SMB2_ERROR_STRUCTURE_SIZE2_LE) &&
+	    smb2_has_data_area(command) &&
+	    len < smb2_min_pdu_len[command]) {
+		cifs_server_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);
@@ -298,33 +338,6 @@ smb2_check_message(char *buf, unsigned int pdu_len, unsigned int len,
 	return 0;
 }
 
-/*
- * The size of the variable area depends on the offset and length fields
- * located in different fields for various SMB2 responses. SMB2 responses
- * with no variable length info, show an offset of zero for the offset field.
- */
-static const bool has_smb2_data_area[NUMBER_OF_SMB2_COMMANDS] = {
-	/* SMB2_NEGOTIATE */ true,
-	/* SMB2_SESSION_SETUP */ true,
-	/* SMB2_LOGOFF */ false,
-	/* SMB2_TREE_CONNECT */	false,
-	/* SMB2_TREE_DISCONNECT */ false,
-	/* SMB2_CREATE */ true,
-	/* SMB2_CLOSE */ false,
-	/* SMB2_FLUSH */ false,
-	/* SMB2_READ */	true,
-	/* SMB2_WRITE */ false,
-	/* SMB2_LOCK */	false,
-	/* SMB2_IOCTL */ true,
-	/* SMB2_CANCEL */ false, /* BB CHECK this not listed in documentation */
-	/* SMB2_ECHO */ false,
-	/* SMB2_QUERY_DIRECTORY */ true,
-	/* SMB2_CHANGE_NOTIFY */ true,
-	/* SMB2_QUERY_INFO */ true,
-	/* SMB2_SET_INFO */ false,
-	/* SMB2_OPLOCK_BREAK */ false
-};
-
 /*
  * Returns the pointer to the beginning of the data area. Length of the data
  * area and the offset to it (from the beginning of the smb are also returned.
@@ -451,7 +464,7 @@ __smb2_calc_size(void *buf, bool *have_data, bool *data_area_overlap)
 	 */
 	len += le16_to_cpu(pdu->StructureSize2);
 
-	if (has_smb2_data_area[le16_to_cpu(shdr->Command)] == false)
+	if (!smb2_has_data_area(le16_to_cpu(shdr->Command)))
 		goto calc_size_exit;
 
 	smb2_get_data_area_len(&offset, &data_length, shdr);
-- 
2.55.0


  parent reply	other threads:[~2026-09-16 21:34 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-16 21:33 [PATCH v5 0/9] smb: client: fix OOB reads and UAFs in SMB2/3 receive paths Frank Sorenson
2026-09-16 21:33 ` [PATCH v5 1/9] smb: client: fix next_buffer UAF and NextCommand bounds in compound PDUs Frank Sorenson
2026-09-16 21:33 ` Frank Sorenson [this message]
2026-09-16 21:33 ` [PATCH v5 3/9] smb: client: fix server->total_read for compound encrypted PDUs Frank Sorenson
2026-09-16 21:33 ` [PATCH v5 4/9] smb: client: fix missing lower-bound check on DFS referral string offsets Frank Sorenson
2026-09-16 21:33 ` [PATCH v5 5/9] smb: client: reject short Next offsets in parse_server_interfaces() Frank Sorenson
2026-09-16 21:33 ` [PATCH v5 6/9] smb: client: fix OOB struct field reads in move_smb2_ea_to_cifs() Frank Sorenson
2026-09-16 21:33 ` [PATCH v5 7/9] smb: client: fix missing iov bounds check in parse_posix_sids() Frank Sorenson
2026-09-16 21:33 ` [PATCH v5 8/9] smb: client: fix potential OOB read in smb3_enum_snapshots() Frank Sorenson
2026-09-16 21:34 ` [PATCH v5 9/9] smb: client: fix reparse buffer bounds in cifs_query_reparse_point() Frank Sorenson
2026-09-17 18:15 ` [PATCH v5 0/9] smb: client: fix OOB reads and UAFs in SMB2/3 receive paths Paulo Alcantara

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=20260916213406.1496960-3-sorenson@redhat.com \
    --to=sorenson@redhat.com \
    --cc=bharathsm@microsoft.com \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=pc@manguebit.org \
    --cc=ronniesahlberg@gmail.com \
    --cc=sprasad@microsoft.com \
    --cc=tom@talpey.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