FAILED: patch "[PATCH] smb: client: validate NT transaction response layout and" failed to apply to 5.10-stable tree

PATCH New [email protected] 2 participants 2 messages Nov 4 – Nov 5

Subsystems (2)

Stable branch SMB client (CIFS)

lore

Share & tools

lore

[email protected]

author maintainer

2026-11-04 18:35 UTC #0 lore raw

The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.10.y
git checkout FETCH_HEAD
git cherry-pick -x 83d0d5e0ca22233c75daf139ec0063b9d2ff1ac7
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026110455-observing-blush-5a37@gregkh' --subject-prefix 'PATCH 5.10.y' 'HEAD^..'

Possible dependencies:

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 83d0d5e0ca22233c75daf139ec0063b9d2ff1ac7 Mon Sep 17 00:00:00 2001
From: Ratnaker Gautam <[email protected]>
Date: Sat, 17 Oct 2026 23:15:05 +0200
Subject: [PATCH] smb: client: validate NT transaction response layout and
offsets

validate_ntransact() assumes that the response uses the fixed 18-word NT transaction layout. A different WordCount makes get_bcc() locate the byte count at a different offset from pSMBr->ByteCount, allowing the subsequent bounds checks to use inconsistent response boundaries.

The function also turns the 32-bit ParameterOffset and DataOffset fields into pointers before checking their bounds. On a 32-bit client, a large offset can wrap the pointer below end_of_smb and pass the one-sided pointer comparison. The caller then dereferences the parameter pointer or copies from the data pointer.

Require the expected WordCount before reading the byte count. Check each offset and count against the received SMB length as integers before forming either pointer. Use subtraction after validating the offset so that the range checks cannot overflow.

Reported-by Ratnaker Gautam <[email protected]>

Fixes 0a4b92c05ed0 ("[CIFS] Add worker function for Get ACL cifs style")

Cc [email protected]

Signed-off-by Ratnaker Gautam <[email protected]>

Reviewed-by Namjae Jeon <[email protected]>

Signed-off-by Paulo Alcantara <[email protected]>

fs/smb/client/cifssmb.c +19 ~~-13~~

index e335686a68f2..9ffc94ac2069 100644
@@ -3787,9 +3787,10 @@ static int
 validate_ntransact(char *buf, char **ppparm, char **ppdata,
 		   __u32 *pparmlen, __u32 *pdatalen)
 {
-	char *end_of_smb;
 	__u32 data_count, data_offset, parm_count, parm_offset;
 	struct smb_com_ntransact_rsp *pSMBr;
+	size_t buflen;
+	char *end_buf;
 	u16 bcc;
 
 	*pdatalen = 0;

@@ -3800,37 +3801,42 @@ validate_ntransact(char *buf, char **ppparm, char **ppdata,
 
 	pSMBr = (struct smb_com_ntransact_rsp *)buf;
 
+	if (pSMBr->hdr.WordCount != 18) {
+		cifs_dbg(FYI, "invalid NT transaction WordCount %u\n",
+			 pSMBr->hdr.WordCount);
+		return -EINVAL;
+	}
+
 	bcc = get_bcc(&pSMBr->hdr);
-	end_of_smb = 2 /* sizeof byte count */ + bcc +
-			(char *)&pSMBr->ByteCount;
+	end_buf = 2 /* sizeof byte count */ + bcc + (char *)&pSMBr->ByteCount;
+	buflen = end_buf - buf;
 
 	data_offset = le32_to_cpu(pSMBr->DataOffset);
 	data_count = le32_to_cpu(pSMBr->DataCount);
 	parm_offset = le32_to_cpu(pSMBr->ParameterOffset);
 	parm_count = le32_to_cpu(pSMBr->ParameterCount);
 
-	*ppparm = (char *)&pSMBr->hdr.Protocol + parm_offset;
-	*ppdata = (char *)&pSMBr->hdr.Protocol + data_offset;
-
 	/* should we also check that parm and data areas do not overlap? */
-	if (*ppparm > end_of_smb) {
+	if (parm_offset > buflen) {
 		cifs_dbg(FYI, "parms start after end of smb\n");
 		return -EINVAL;
-	} else if (parm_count + *ppparm > end_of_smb) {
+	} else if (parm_count > buflen - parm_offset) {
 		cifs_dbg(FYI, "parm end after end of smb\n");
 		return -EINVAL;
-	} else if (*ppdata > end_of_smb) {
+	} else if (data_offset > buflen) {
 		cifs_dbg(FYI, "data starts after end of smb\n");
 		return -EINVAL;
-	} else if (data_count + *ppdata > end_of_smb) {
-		cifs_dbg(FYI, "data %p + count %d (%p) past smb end %p start %p\n",
-			 *ppdata, data_count, (data_count + *ppdata),
-			 end_of_smb, pSMBr);
+	} else if (data_count > buflen - data_offset) {
+		cifs_dbg(FYI, "data offset %u + count %u past smb length %zu\n",
+			 data_offset, data_count, buflen);
 		return -EINVAL;
 	} else if (parm_count + data_count > bcc) {
 		cifs_dbg(FYI, "parm count and data count larger than SMB\n");
 		return -EINVAL;
 	}
+
+	*ppparm = buf + parm_offset;
+	*ppdata = buf + data_offset;
 	*pdatalen = data_count;
 	*pparmlen = parm_count;
 	return 0;

Sasha Levin

maintainer

2026-11-05 19:42 UTC #1 lore raw

PATCH 1/1 smb: client: validate NT transaction response layout and offsets

From: Ratnaker Gautam <[email protected]>

[ Upstream commit 83d0d5e0ca22233c75daf139ec0063b9d2ff1ac7 ]

Reported-by Ratnaker Gautam <[email protected]>

Fixes 0a4b92c05ed0 ("[CIFS] Add worker function for Get ACL cifs style")

Cc [email protected]

Signed-off-by Ratnaker Gautam <[email protected]>

Reviewed-by Namjae Jeon <[email protected]>

Signed-off-by Paulo Alcantara <[email protected]>

Signed-off-by Sasha Levin <[email protected]>

---
 fs/cifs/cifssmb.c | 32 +++++++++++++++++++-------------
 1 file changed, 19 insertions(+), 13 deletions(-)

fs/cifs/cifssmb.c +19 ~~-13~~

index 67ff1669cab2a..9af7ddd21d4f1 100644
@@ -3843,9 +3843,10 @@ static int
 validate_ntransact(char *buf, char **ppparm, char **ppdata,
 		   __u32 *pparmlen, __u32 *pdatalen)
 {
-	char *end_of_smb;
 	__u32 data_count, data_offset, parm_count, parm_offset;
 	struct smb_com_ntransact_rsp *pSMBr;
+	size_t buflen;
+	char *end_buf;
 	u16 bcc;
 
 	*pdatalen = 0;

@@ -3856,37 +3857,42 @@ validate_ntransact(char *buf, char **ppparm, char **ppdata,
 
 	pSMBr = (struct smb_com_ntransact_rsp *)buf;
 
+	if (pSMBr->hdr.WordCount != 18) {
+		cifs_dbg(FYI, "invalid NT transaction WordCount %u\n",
+			 pSMBr->hdr.WordCount);
+		return -EINVAL;
+	}
+
 	bcc = get_bcc(&pSMBr->hdr);
-	end_of_smb = 2 /* sizeof byte count */ + bcc +
-			(char *)&pSMBr->ByteCount;
+	end_buf = 2 /* sizeof byte count */ + bcc + (char *)&pSMBr->ByteCount;
+	buflen = end_buf - buf;
 
 	data_offset = le32_to_cpu(pSMBr->DataOffset);
 	data_count = le32_to_cpu(pSMBr->DataCount);
 	parm_offset = le32_to_cpu(pSMBr->ParameterOffset);
 	parm_count = le32_to_cpu(pSMBr->ParameterCount);
 
-	*ppparm = (char *)&pSMBr->hdr.Protocol + parm_offset;
-	*ppdata = (char *)&pSMBr->hdr.Protocol + data_offset;
-
 	/* should we also check that parm and data areas do not overlap? */
-	if (*ppparm > end_of_smb) {
+	if (parm_offset > buflen) {
 		cifs_dbg(FYI, "parms start after end of smb\n");
 		return -EINVAL;
-	} else if (parm_count + *ppparm > end_of_smb) {
+	} else if (parm_count > buflen - parm_offset) {
 		cifs_dbg(FYI, "parm end after end of smb\n");
 		return -EINVAL;
-	} else if (*ppdata > end_of_smb) {
+	} else if (data_offset > buflen) {
 		cifs_dbg(FYI, "data starts after end of smb\n");
 		return -EINVAL;
-	} else if (data_count + *ppdata > end_of_smb) {
-		cifs_dbg(FYI, "data %p + count %d (%p) past smb end %p start %p\n",
-			 *ppdata, data_count, (data_count + *ppdata),
-			 end_of_smb, pSMBr);
+	} else if (data_count > buflen - data_offset) {
+		cifs_dbg(FYI, "data offset %u + count %u past smb length %zu\n",
+			 data_offset, data_count, buflen);
 		return -EINVAL;
 	} else if (parm_count + data_count > bcc) {
 		cifs_dbg(FYI, "parm count and data count larger than SMB\n");
 		return -EINVAL;
 	}
+
+	*ppparm = buf + parm_offset;
+	*ppdata = buf + data_offset;
 	*pdatalen = data_count;
 	*pparmlen = parm_count;
 	return 0;

Signature · 1 line

2.53.0