[PATCH 6.12 596/666] smb: client: fix OOB reads parsing symlink error response

[PATCH 6.12 596/666] smb: client: fix OOB reads parsing symlink error response

[Greg Kroah-Hartman]

6.12-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[ Upstream commit 3df690bba28edec865cf7190be10708ad0ddd67e ]

When a CREATE returns STATUS_STOPPED_ON_SYMLINK, smb2_check_message()
returns success without any length validation, leaving the symlink
parsers as the only defense against an untrusted server.

symlink_data() walks SMB 3.1.1 error contexts with the loop test "p <
end", but reads p->ErrorId at offset 4 and p->ErrorDataLength at offset
0.  When the server-controlled ErrorDataLength advances p to within 1-7
bytes of end, the next iteration will read past it.  When the matching
context is found, sym->SymLinkErrorTag is read at offset 4 from
p->ErrorContextData with no check that the symlink header itself fits.

smb2_parse_symlink_response() then bounds-checks the substitute name
using SMB2_SYMLINK_STRUCT_SIZE as the offset of PathBuffer from
iov_base.  That value is computed as sizeof(smb2_err_rsp) +
sizeof(smb2_symlink_err_rsp), which is correct only when
ErrorContextCount == 0.

With at least one error context the symlink data sits 8 bytes deeper,
and each skipped non-matching context shifts it further by 8 +
ALIGN(ErrorDataLength, 8).  The check is too short, allowing the
substitute name read to run past iov_len.  The out-of-bound heap bytes
are UTF-16-decoded into the symlink target and returned to userspace via
readlink(2).

Fix this all up by making the loops test require the full context header
to fit, rejecting sym if its header runs past end, and bound the
substitute name against the actual position of sym->PathBuffer rather
than a fixed offset.

Because sub_offs and sub_len are 16bits, the pointer math will not
overflow here with the new greater-than.

Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Cc: stable <stable@kernel.org>
Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Alva Lan <alvalan9@foxmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 fs/smb/client/smb2file.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/fs/smb/client/smb2file.c b/fs/smb/client/smb2file.c
index 35d2933982d31..fd331a9f2f4d2 100644
--- a/fs/smb/client/smb2file.c
+++ b/fs/smb/client/smb2file.c
@@ -27,10 +27,11 @@ static struct smb2_symlink_err_rsp *symlink_data(const struct kvec *iov)
 {
 	struct smb2_err_rsp *err = iov->iov_base;
 	struct smb2_symlink_err_rsp *sym = ERR_PTR(-EINVAL);
+	u8 *end = (u8 *)err + iov->iov_len;
 	u32 len;
 
 	if (err->ErrorContextCount) {
-		struct smb2_error_context_rsp *p, *end;
+		struct smb2_error_context_rsp *p;
 
 		len = (u32)err->ErrorContextCount * (offsetof(struct smb2_error_context_rsp,
 							      ErrorContextData) +
@@ -39,8 +40,7 @@ static struct smb2_symlink_err_rsp *symlink_data(const struct kvec *iov)
 			return ERR_PTR(-EINVAL);
 
 		p = (struct smb2_error_context_rsp *)err->ErrorData;
-		end = (struct smb2_error_context_rsp *)((u8 *)err + iov->iov_len);
-		do {
+		while ((u8 *)p + sizeof(*p) <= end) {
 			if (le32_to_cpu(p->ErrorId) == SMB2_ERROR_ID_DEFAULT) {
 				sym = (struct smb2_symlink_err_rsp *)p->ErrorContextData;
 				break;
@@ -50,14 +50,16 @@ static struct smb2_symlink_err_rsp *symlink_data(const struct kvec *iov)
 
 			len = ALIGN(le32_to_cpu(p->ErrorDataLength), 8);
 			p = (struct smb2_error_context_rsp *)(p->ErrorContextData + len);
-		} while (p < end);
+		}
 	} else if (le32_to_cpu(err->ByteCount) >= sizeof(*sym) &&
 		   iov->iov_len >= SMB2_SYMLINK_STRUCT_SIZE) {
 		sym = (struct smb2_symlink_err_rsp *)err->ErrorData;
 	}
 
-	if (!IS_ERR(sym) && (le32_to_cpu(sym->SymLinkErrorTag) != SYMLINK_ERROR_TAG ||
-			     le32_to_cpu(sym->ReparseTag) != IO_REPARSE_TAG_SYMLINK))
+	if (!IS_ERR(sym) &&
+	    ((u8 *)sym + sizeof(*sym) > end ||
+	     le32_to_cpu(sym->SymLinkErrorTag) != SYMLINK_ERROR_TAG ||
+	     le32_to_cpu(sym->ReparseTag) != IO_REPARSE_TAG_SYMLINK))
 		sym = ERR_PTR(-EINVAL);
 
 	return sym;
@@ -82,8 +84,10 @@ int smb2_parse_symlink_response(struct cifs_sb_info *cifs_sb, const struct kvec
 	print_len = le16_to_cpu(sym->PrintNameLength);
 	print_offs = le16_to_cpu(sym->PrintNameOffset);
 
-	if (iov->iov_len < SMB2_SYMLINK_STRUCT_SIZE + sub_offs + sub_len ||
-	    iov->iov_len < SMB2_SYMLINK_STRUCT_SIZE + print_offs + print_len)
+	if ((char *)sym->PathBuffer + sub_offs + sub_len >
+		(char *)iov->iov_base + iov->iov_len ||
+	    (char *)sym->PathBuffer + print_offs + print_len >
+		(char *)iov->iov_base + iov->iov_len)
 		return -EINVAL;
 
 	return smb2_parse_native_symlink(path,
-- 
2.53.0

[PATCH 6.12 634/666] netfs: fix error handling in netfs_extract_user_iter()

[Greg Kroah-Hartman]

6.12-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Paulo Alcantara <pc@manguebit.org>

commit 0aad5704c6b4d14007d4eab15883e8524e4310f4 upstream.

In netfs_extract_user_iter(), if iov_iter_extract_pages() failed to
extract user pages, bail out on -ENOMEM, otherwise return the error
code only if @npages == 0, allowing short DIO reads and writes to be
issued.

This fixes mmapstress02 from LTP tests against CIFS.

Fixes: 85dd2c8ff368 ("netfs: Add a function to extract a UBUF or IOVEC into a BVEC iterator")
Reported-by: Xiaoli Feng <xifeng@redhat.com>
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Link: https://patch.msgid.link/20260512123404.719402-10-dhowells@redhat.com
Cc: netfs@lists.linux.dev
Cc: stable@vger.kernel.org
Cc: linux-cifs@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/netfs/iterator.c |   13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

--- a/fs/netfs/iterator.c
+++ b/fs/netfs/iterator.c
@@ -22,7 +22,7 @@
  *
  * Extract the page fragments from the given amount of the source iterator and
  * build up a second iterator that refers to all of those bits.  This allows
- * the original iterator to disposed of.
+ * the original iterator to be disposed of.
  *
  * @extraction_flags can have ITER_ALLOW_P2PDMA set to request peer-to-peer DMA be
  * allowed on the pages extracted.
@@ -67,8 +67,8 @@ ssize_t netfs_extract_user_iter(struct i
 		ret = iov_iter_extract_pages(orig, &pages, count,
 					     max_pages - npages, extraction_flags,
 					     &offset);
-		if (ret < 0) {
-			pr_err("Couldn't get user pages (rc=%zd)\n", ret);
+		if (unlikely(ret <= 0)) {
+			ret = ret ?: -EIO;
 			break;
 		}
 
@@ -97,6 +97,13 @@ ssize_t netfs_extract_user_iter(struct i
 		npages += cur_npages;
 	}
 
+	if (ret < 0 && (ret == -ENOMEM || npages == 0)) {
+		for (i = 0; i < npages; i++)
+			unpin_user_page(bv[i].bv_page);
+		kvfree(bv);
+		return ret;
+	}
+
 	iov_iter_bvec(new, orig->data_source, bv, npages, orig_len - count);
 	return npages;
 }

Re: [PATCH 6.12 634/666] netfs: fix error handling in netfs_extract_user_iter()

[Harshit Mogalapalli]

Hi Greg/Sasha,

On 20/05/26 21:54, Greg Kroah-Hartman wrote:
> 6.12-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Paulo Alcantara <pc@manguebit.org>
> 
> commit 0aad5704c6b4d14007d4eab15883e8524e4310f4 upstream.
> 
> In netfs_extract_user_iter(), if iov_iter_extract_pages() failed to
> extract user pages, bail out on -ENOMEM, otherwise return the error
> code only if @npages == 0, allowing short DIO reads and writes to be
> issued.
> 
> This fixes mmapstress02 from LTP tests against CIFS.
> 
> Fixes: 85dd2c8ff368 ("netfs: Add a function to extract a UBUF or IOVEC into a BVEC iterator")
> Reported-by: Xiaoli Feng <xifeng@redhat.com>
> Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Link: https://patch.msgid.link/20260512123404.719402-10-dhowells@redhat.com
> Cc: netfs@lists.linux.dev
> Cc: stable@vger.kernel.org
> Cc: linux-cifs@vger.kernel.org
> Cc: linux-fsdevel@vger.kernel.org
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>   fs/netfs/iterator.c |   13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
> 
> --- a/fs/netfs/iterator.c
> +++ b/fs/netfs/iterator.c
> @@ -22,7 +22,7 @@
>    *
>    * Extract the page fragments from the given amount of the source iterator and
>    * build up a second iterator that refers to all of those bits.  This allows
> - * the original iterator to disposed of.
> + * the original iterator to be disposed of.
>    *
>    * @extraction_flags can have ITER_ALLOW_P2PDMA set to request peer-to-peer DMA be
>    * allowed on the pages extracted.
> @@ -67,8 +67,8 @@ ssize_t netfs_extract_user_iter(struct i
>   		ret = iov_iter_extract_pages(orig, &pages, count,
>   					     max_pages - npages, extraction_flags,
>   					     &offset);
> -		if (ret < 0) {
> -			pr_err("Couldn't get user pages (rc=%zd)\n", ret);
> +		if (unlikely(ret <= 0)) {
> +			ret = ret ?: -EIO;
>   			break;
>   		}
>   
> @@ -97,6 +97,13 @@ ssize_t netfs_extract_user_iter(struct i
>   		npages += cur_npages;
>   	}
>   
> +	if (ret < 0 && (ret == -ENOMEM || npages == 0)) {
> +		for (i = 0; i < npages; i++)
> +			unpin_user_page(bv[i].bv_page);
> +		kvfree(bv);
> +		return ret;
> +	}
> +

I have run an AI assisted backport review and it spotted an issue: I
have taken a look and the issues goes like:

Upstream has:



ssize_t ret = 0;

...
if (ret < 0 && (ret == -ENOMEM || npages == 0)) {
         for (i = 0; i < npages; i++)
                 unpin_user_page(bv[i].bv_page);
         kvfree(bv);
         return ret;
}

6.12.y has:

ssize_t ret;

...
if (ret < 0 && (ret == -ENOMEM || npages == 0)) {
         for (i = 0; i < npages; i++)
                 unpin_user_page(bv[i].bv_page);
         kvfree(bv);
         return ret;
}

I think 6.12.y misses commit: 7e3d8db899d5 ("netfs: Fix potential 
uninitialised var in netfs_extract_user_iter()") so backport might not 
be complete, thoughts ?

thanks,
Harshit


>   	iov_iter_bvec(new, orig->data_source, bv, npages, orig_len - count);
>   	return npages;
>   }
> 
> 
> 

Re: [PATCH 6.12 634/666] netfs: fix error handling in netfs_extract_user_iter()

[Greg Kroah-Hartman]

On Sat, May 23, 2026 at 01:53:15AM +0530, Harshit Mogalapalli wrote:
> Hi Greg/Sasha,
> 
> On 20/05/26 21:54, Greg Kroah-Hartman wrote:
> > 6.12-stable review patch.  If anyone has any objections, please let me know.
> > 
> > ------------------
> > 
> > From: Paulo Alcantara <pc@manguebit.org>
> > 
> > commit 0aad5704c6b4d14007d4eab15883e8524e4310f4 upstream.
> > 
> > In netfs_extract_user_iter(), if iov_iter_extract_pages() failed to
> > extract user pages, bail out on -ENOMEM, otherwise return the error
> > code only if @npages == 0, allowing short DIO reads and writes to be
> > issued.
> > 
> > This fixes mmapstress02 from LTP tests against CIFS.
> > 
> > Fixes: 85dd2c8ff368 ("netfs: Add a function to extract a UBUF or IOVEC into a BVEC iterator")
> > Reported-by: Xiaoli Feng <xifeng@redhat.com>
> > Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
> > Signed-off-by: David Howells <dhowells@redhat.com>
> > Link: https://patch.msgid.link/20260512123404.719402-10-dhowells@redhat.com
> > Cc: netfs@lists.linux.dev
> > Cc: stable@vger.kernel.org
> > Cc: linux-cifs@vger.kernel.org
> > Cc: linux-fsdevel@vger.kernel.org
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >   fs/netfs/iterator.c |   13 ++++++++++---
> >   1 file changed, 10 insertions(+), 3 deletions(-)
> > 
> > --- a/fs/netfs/iterator.c
> > +++ b/fs/netfs/iterator.c
> > @@ -22,7 +22,7 @@
> >    *
> >    * Extract the page fragments from the given amount of the source iterator and
> >    * build up a second iterator that refers to all of those bits.  This allows
> > - * the original iterator to disposed of.
> > + * the original iterator to be disposed of.
> >    *
> >    * @extraction_flags can have ITER_ALLOW_P2PDMA set to request peer-to-peer DMA be
> >    * allowed on the pages extracted.
> > @@ -67,8 +67,8 @@ ssize_t netfs_extract_user_iter(struct i
> >   		ret = iov_iter_extract_pages(orig, &pages, count,
> >   					     max_pages - npages, extraction_flags,
> >   					     &offset);
> > -		if (ret < 0) {
> > -			pr_err("Couldn't get user pages (rc=%zd)\n", ret);
> > +		if (unlikely(ret <= 0)) {
> > +			ret = ret ?: -EIO;
> >   			break;
> >   		}
> > @@ -97,6 +97,13 @@ ssize_t netfs_extract_user_iter(struct i
> >   		npages += cur_npages;
> >   	}
> > +	if (ret < 0 && (ret == -ENOMEM || npages == 0)) {
> > +		for (i = 0; i < npages; i++)
> > +			unpin_user_page(bv[i].bv_page);
> > +		kvfree(bv);
> > +		return ret;
> > +	}
> > +
> 
> I have run an AI assisted backport review and it spotted an issue: I
> have taken a look and the issues goes like:
> 
> Upstream has:
> 
> 
> 
> ssize_t ret = 0;
> 
> ...
> if (ret < 0 && (ret == -ENOMEM || npages == 0)) {
>         for (i = 0; i < npages; i++)
>                 unpin_user_page(bv[i].bv_page);
>         kvfree(bv);
>         return ret;
> }
> 
> 6.12.y has:
> 
> ssize_t ret;
> 
> ...
> if (ret < 0 && (ret == -ENOMEM || npages == 0)) {
>         for (i = 0; i < npages; i++)
>                 unpin_user_page(bv[i].bv_page);
>         kvfree(bv);
>         return ret;
> }
> 
> I think 6.12.y misses commit: 7e3d8db899d5 ("netfs: Fix potential
> uninitialised var in netfs_extract_user_iter()") so backport might not be
> complete, thoughts ?

Now applied, thanks.

greg k-h