[PATCH 0/2] smb: some potential bugfixes

[PATCH 0/2] smb: some potential bugfixes

[Greg Kroah-Hartman]

I spent the time exercising some new fuzzing tools on the ksmbd and smb
code purely because it's something that is simple to set up and test
locally with virtual machines, and in doing so, potentially found some
minor problems for when you have an "untrusted" client.

Here's some fixes for what I happened to notice.  They pass my very
limited testing here, but please don't trust them at all and verify that
I'm not just making this all up before accepting them.

thanks!

greg k-h

Greg Kroah-Hartman (2):
  smb: client: fix off-by-8 bounds check in check_wsl_eas()
  smb: client: fix OOB reads parsing symlink error response

 fs/smb/client/smb2file.c  | 20 ++++++++++++--------
 fs/smb/client/smb2inode.c |  2 +-
 2 files changed, 13 insertions(+), 9 deletions(-)

-- 
2.53.0

[PATCH 1/2] smb: client: fix off-by-8 bounds check in check_wsl_eas()

[Greg Kroah-Hartman]

The bounds check uses (u8 *)ea + nlen + 1 + vlen as the end of the EA
name and value, but ea_data sits at offset sizeof(struct
smb2_file_full_ea_info) = 8 from ea, not at offset 0.  The strncmp()
later reads ea->ea_data[0..nlen-1] and the value bytes follow at
ea_data[nlen+1..nlen+vlen], so the actual end is ea->ea_data + nlen + 1
+ vlen.  Isn't pointer math fun?

The earlier check (u8 *)ea > end - sizeof(*ea) only guarantees the
8-byte header is in bounds, but since the last EA is placed within 8
bytes of the end of the response, the name and value bytes are read past
the end of iov.

Fix this mess all up by using ea->ea_data as the base for the bounds
check.

An "untrusted" server can use this to leak up to 8 bytes of kernel heap
into the EA name comparison and influence which WSL xattr the data is
interpreted as.

Cc: Steve French <sfrench@samba.org>
Cc: Paulo Alcantara <pc@manguebit.org>
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>
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 fs/smb/client/smb2inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index 364bdcff9c9d..fe1c9d776580 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -128,7 +128,7 @@ static int check_wsl_eas(struct kvec *rsp_iov)
 		nlen = ea->ea_name_length;
 		vlen = le16_to_cpu(ea->ea_value_length);
 		if (nlen != SMB2_WSL_XATTR_NAME_LEN ||
-		    (u8 *)ea + nlen + 1 + vlen > end)
+		    (u8 *)ea->ea_data + nlen + 1 + vlen > end)
 			return -EINVAL;
 
 		switch (vlen) {
-- 
2.53.0

Re: [PATCH 1/2] smb: client: fix off-by-8 bounds check in check_wsl_eas()

[ChenXiaoSong]

Sashiko reported another out-of-bounds issue: 
https://sashiko.dev/#/patchset/2026040635-banking-unsoiled-3250@gregkh

Should we add the following checks in check_wsl_eas()?

```
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -121,6 +121,9 @@ static int check_wsl_eas(struct kvec *rsp_iov)
         ea = (void *)((u8 *)rsp_iov->iov_base +
                       le16_to_cpu(rsp->OutputBufferOffset));
         end = (u8 *)rsp_iov->iov_base + rsp_iov->iov_len;
+       if (ea + outlen > end)
+               return -EINVAL;
+
         for (;;) {
                 if ((u8 *)ea > end - sizeof(*ea))
                         return -EINVAL;
```

On 4/6/26 21:49, Greg Kroah-Hartman wrote:
> The bounds check uses (u8 *)ea + nlen + 1 + vlen as the end of the EA
> name and value, but ea_data sits at offset sizeof(struct
> smb2_file_full_ea_info) = 8 from ea, not at offset 0.  The strncmp()
> later reads ea->ea_data[0..nlen-1] and the value bytes follow at
> ea_data[nlen+1..nlen+vlen], so the actual end is ea->ea_data + nlen + 1
> + vlen.  Isn't pointer math fun?
> 
> The earlier check (u8 *)ea > end - sizeof(*ea) only guarantees the
> 8-byte header is in bounds, but since the last EA is placed within 8
> bytes of the end of the response, the name and value bytes are read past
> the end of iov.
> 
> Fix this mess all up by using ea->ea_data as the base for the bounds
> check.
> 
> An "untrusted" server can use this to leak up to 8 bytes of kernel heap
> into the EA name comparison and influence which WSL xattr the data is
> interpreted as.
> 
> Cc: Steve French <sfrench@samba.org>
> Cc: Paulo Alcantara <pc@manguebit.org>
> 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>
> Assisted-by: gregkh_clanker_t1000
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>   fs/smb/client/smb2inode.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> index 364bdcff9c9d..fe1c9d776580 100644
> --- a/fs/smb/client/smb2inode.c
> +++ b/fs/smb/client/smb2inode.c
> @@ -128,7 +128,7 @@ static int check_wsl_eas(struct kvec *rsp_iov)
>   		nlen = ea->ea_name_length;
>   		vlen = le16_to_cpu(ea->ea_value_length);
>   		if (nlen != SMB2_WSL_XATTR_NAME_LEN ||
> -		    (u8 *)ea + nlen + 1 + vlen > end)
> +		    (u8 *)ea->ea_data + nlen + 1 + vlen > end)
>   			return -EINVAL;
>   
>   		switch (vlen) {

-- 
ChenXiaoSong <chenxiaosong@kylinos.cn>
Chinese Homepage: chenxiaosong.com
English Homepage: chenxiaosong.com/en

Re: [PATCH 1/2] smb: client: fix off-by-8 bounds check in check_wsl_eas()

[Greg Kroah-Hartman]

On Wed, Apr 08, 2026 at 10:51:12AM +0800, ChenXiaoSong wrote:
> Sashiko reported another out-of-bounds issue:
> https://sashiko.dev/#/patchset/2026040635-banking-unsoiled-3250@gregkh
> 
> Should we add the following checks in check_wsl_eas()?
> 
> ```
> --- a/fs/smb/client/smb2inode.c
> +++ b/fs/smb/client/smb2inode.c
> @@ -121,6 +121,9 @@ static int check_wsl_eas(struct kvec *rsp_iov)
>         ea = (void *)((u8 *)rsp_iov->iov_base +
>                       le16_to_cpu(rsp->OutputBufferOffset));
>         end = (u8 *)rsp_iov->iov_base + rsp_iov->iov_len;
> +       if (ea + outlen > end)
> +               return -EINVAL;

Then you would miss any "first" structures here, as I think the for loop
catches this later on with the line:


> +
>         for (;;) {
>                 if ((u8 *)ea > end - sizeof(*ea))
>                         return -EINVAL;

That one, right?

Or am I misreading this?

Pointer math is "fun" :(

thanks,

greg k-h

Re: [PATCH 1/2] smb: client: fix off-by-8 bounds check in check_wsl_eas()

[ChenXiaoSong]

The for loop does not seem to catch cases where `outlen` is excessively 
large. In such cases, smb2_compound_op() would use this large `outlen` 
to `memcpy()`, which could lead to OOB.

```
smb2_compound_op()
{
	...
	size[0] = outlen; // very large
	check_wsl_eas()
	memcpy(..., outlen) // out-of-bounds
	...
}
```

On 4/8/26 13:39, Greg Kroah-Hartman wrote:
> On Wed, Apr 08, 2026 at 10:51:12AM +0800, ChenXiaoSong wrote:
>> Sashiko reported another out-of-bounds issue:
>> https://sashiko.dev/#/patchset/2026040635-banking-unsoiled-3250@gregkh
>>
>> Should we add the following checks in check_wsl_eas()?
>>
>> ```
>> --- a/fs/smb/client/smb2inode.c
>> +++ b/fs/smb/client/smb2inode.c
>> @@ -121,6 +121,9 @@ static int check_wsl_eas(struct kvec *rsp_iov)
>>          ea = (void *)((u8 *)rsp_iov->iov_base +
>>                        le16_to_cpu(rsp->OutputBufferOffset));
>>          end = (u8 *)rsp_iov->iov_base + rsp_iov->iov_len;
>> +       if (ea + outlen > end)
>> +               return -EINVAL;
> 
> Then you would miss any "first" structures here, as I think the for loop
> catches this later on with the line:
> 
> 
>> +
>>          for (;;) {
>>                  if ((u8 *)ea > end - sizeof(*ea))
>>                          return -EINVAL;
> 
> That one, right?
> 
> Or am I misreading this?
> 
> Pointer math is "fun" :(
> 
> thanks,
> 
> greg k-h

-- 
ChenXiaoSong <chenxiaosong@kylinos.cn>
Chinese Homepage: chenxiaosong.com
English Homepage: chenxiaosong.com/en

Re: [PATCH 1/2] smb: client: fix off-by-8 bounds check in check_wsl_eas()

[Greg Kroah-Hartman]

On Wed, Apr 08, 2026 at 01:58:27PM +0800, ChenXiaoSong wrote:
> The for loop does not seem to catch cases where `outlen` is excessively
> large. In such cases, smb2_compound_op() would use this large `outlen` to
> `memcpy()`, which could lead to OOB.
> 
> ```
> smb2_compound_op()
> {
> 	...
> 	size[0] = outlen; // very large
> 	check_wsl_eas()
> 	memcpy(..., outlen) // out-of-bounds
> 	...
> }
> ```

Ah, I missed the caller site.  Yeah, probably a good thing to check as
well, want to make up a patch?

thanks,
greg k-h

Re: [PATCH 1/2] smb: client: fix off-by-8 bounds check in check_wsl_eas()

[ChenXiaoSong]

Could you send v2 with this check added?

I am currently reviewing another patch. Perhaps you could wait until I 
have finished reviewing it.

On 4/8/26 14:15, Greg Kroah-Hartman wrote:
> Ah, I missed the caller site.  Yeah, probably a good thing to check as
> well, want to make up a patch?

-- 
ChenXiaoSong <chenxiaosong@kylinos.cn>
Chinese Homepage: chenxiaosong.com
English Homepage: chenxiaosong.com/en

Re: [PATCH 1/2] smb: client: fix off-by-8 bounds check in check_wsl_eas()

[Greg Kroah-Hartman]

On Wed, Apr 08, 2026 at 02:19:48PM +0800, ChenXiaoSong wrote:
> Could you send v2 with this check added?

Looks like this was already accepted, and the additional check would be
just that, an additional check :)

> I am currently reviewing another patch. Perhaps you could wait until I have
> finished reviewing it.

I have no problem waiting, but as you found this one, I want to give you
proper credit for the find/fix.

thanks,

greg k-h

Re: [PATCH 1/2] smb: client: fix off-by-8 bounds check in check_wsl_eas()

[ChenXiaoSong]

Sashiko found it, I just checked and confirmed it.

Could you create another patch? I enjoy the review process more than 
creating patches :)

On 4/8/26 14:40, Greg Kroah-Hartman wrote:
> On Wed, Apr 08, 2026 at 02:19:48PM +0800, ChenXiaoSong wrote:
>> Could you send v2 with this check added?
> 
> Looks like this was already accepted, and the additional check would be
> just that, an additional check :)
> 
>> I am currently reviewing another patch. Perhaps you could wait until I have
>> finished reviewing it.
> 
> I have no problem waiting, but as you found this one, I want to give you
> proper credit for the find/fix.
> 
> thanks,
> 
> greg k-h

-- 
ChenXiaoSong <chenxiaosong@kylinos.cn>
Chinese Homepage: chenxiaosong.com
English Homepage: chenxiaosong.com/en

Re: [PATCH 1/2] smb: client: fix off-by-8 bounds check in check_wsl_eas()

[Greg Kroah-Hartman]

On Wed, Apr 08, 2026 at 02:52:31PM +0800, ChenXiaoSong wrote:
> Sashiko found it, I just checked and confirmed it.
> 
> Could you create another patch? I enjoy the review process more than
> creating patches :)

Will do, I'll add that to my queue, but might take a few days if someone
wants to "beat" me to it, I'll have no objection at all :)

thanks,

greg k-h

Re: [PATCH 1/2] smb: client: fix off-by-8 bounds check in check_wsl_eas()

[ChenXiaoSong]

Hi Steve,

Should we use `get_unaligned_le32()` to get unaligned data?

> static inline kuid_t wsl_make_kuid(struct cifs_sb_info *cifs_sb,
>                                    void *ptr)                   
> {                                                               
>         u32 uid = le32_to_cpu(*(__le32 *)ptr);
>         ...
> }                                                               


> 
> Additionally, does parsing these extended attributes cause unaligned memory
> accesses?
> When parsing WSL extended attributes, the code derives the value pointer
> at an offset of ea_name_length (which is 6) plus 1 from ea_data. Since
> ea is a 4-byte aligned structure, the value sits at an unaligned offset.
> Helper functions like wsl_make_kuid() explicitly cast this unaligned
> pointer to a 32-bit type and dereference it:
>     le32_to_cpu(*(__le32 *)v);
>     
> Could this trigger an unaligned access exception and crash the kernel on
> architectures with strict alignment requirements?

-- 
ChenXiaoSong <chenxiaosong@kylinos.cn>
Chinese Homepage: chenxiaosong.com
English Homepage: chenxiaosong.com/en

Re: [PATCH 1/2] smb: client: fix off-by-8 bounds check in check_wsl_eas()

[Yuanfu Xie]

[PATCH] smb: client: add bounds checks when iterating SMB1 WSL EAs

Add boundary and overflow checks when traversing extended attribute (EA)
entries for SMB1 WSL reparse points in cifs_query_path_info(), to prevent
out-of-bounds memory accesses from malformed next_entry_offset values.

This is necessary because SMB1 WSL EA iteration does not currently have
any explicit bounds checking, unlike SMB2 path which has check_wsl_eas().
The new checks ensure the current EA pointer plus its size does not
exceed the end of the EA buffer during iteration and before parsing.

Signed-off-by: YuanfuXie <yuanfuxie@stu.pku.edu.cn>
---
 fs/smb/client/smb1ops.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c
index 9694117050a6..940bae49b2cc 100644
--- a/fs/smb/client/smb1ops.c
+++ b/fs/smb/client/smb1ops.c
@@ -645,13 +645,17 @@ static int cifs_query_path_info(const unsigned int xid,
 	if (!rc && data->reparse_point) {
 		struct smb2_file_full_ea_info *ea;
 		u32 next = 0;
+		u8 *eas_end = data->wsl.eas + data->wsl.eas_len;
 
 		ea = (struct smb2_file_full_ea_info *)data->wsl.eas;
 		do {
 			ea = (void *)((u8 *)ea + next);
+			if ((u8 *)ea + sizeof(*ea) > eas_end)
+				break;
 			next = le32_to_cpu(ea->next_entry_offset);
 		} while (next);
-		if (le16_to_cpu(ea->ea_value_length)) {
+		if ((u8 *)ea + sizeof(*ea) <= eas_end &&
+		    le16_to_cpu(ea->ea_value_length)) {
 			ea->next_entry_offset = cpu_to_le32(ALIGN(sizeof(*ea) +
 						ea->ea_name_length + 1 +
 						le16_to_cpu(ea->ea_value_length), 4));
@@ -691,13 +695,17 @@ static int cifs_query_path_info(const unsigned int xid,
 	if (!rc && data->reparse_point) {
 		struct smb2_file_full_ea_info *ea;
 		u32 next = 0;
+		u8 *eas_end = data->wsl.eas + data->wsl.eas_len;
 
 		ea = (struct smb2_file_full_ea_info *)data->wsl.eas;
 		do {
 			ea = (void *)((u8 *)ea + next);
+			if ((u8 *)ea + sizeof(*ea) > eas_end)
+				break;
 			next = le32_to_cpu(ea->next_entry_offset);
 		} while (next);
-		if (le16_to_cpu(ea->ea_value_length)) {
+		if ((u8 *)ea + sizeof(*ea) <= eas_end &&
+		    le16_to_cpu(ea->ea_value_length)) {
 			ea->next_entry_offset = cpu_to_le32(ALIGN(sizeof(*ea) +
 						ea->ea_name_length + 1 +
 						le16_to_cpu(ea->ea_value_length), 4));
-- 
2.43.0

> On Apr 6, 2026, at 21:49, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> The bounds check uses (u8 *)ea + nlen + 1 + vlen as the end of the EA
> name and value, but ea_data sits at offset sizeof(struct
> smb2_file_full_ea_info) = 8 from ea, not at offset 0.  The strncmp()
> later reads ea->ea_data[0..nlen-1] and the value bytes follow at
> ea_data[nlen+1..nlen+vlen], so the actual end is ea->ea_data + nlen + 1
> + vlen.  Isn't pointer math fun?
> 
> The earlier check (u8 *)ea > end - sizeof(*ea) only guarantees the
> 8-byte header is in bounds, but since the last EA is placed within 8
> bytes of the end of the response, the name and value bytes are read past
> the end of iov.
> 
> Fix this mess all up by using ea->ea_data as the base for the bounds
> check.
> 
> An "untrusted" server can use this to leak up to 8 bytes of kernel heap
> into the EA name comparison and influence which WSL xattr the data is
> interpreted as.
> 
> Cc: Steve French <sfrench@samba.org>
> Cc: Paulo Alcantara <pc@manguebit.org>
> 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>
> Assisted-by: gregkh_clanker_t1000
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> fs/smb/client/smb2inode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> index 364bdcff9c9d..fe1c9d776580 100644
> --- a/fs/smb/client/smb2inode.c
> +++ b/fs/smb/client/smb2inode.c
> @@ -128,7 +128,7 @@ static int check_wsl_eas(struct kvec *rsp_iov)
> 		nlen = ea->ea_name_length;
> 		vlen = le16_to_cpu(ea->ea_value_length);
> 		if (nlen != SMB2_WSL_XATTR_NAME_LEN ||
> -		    (u8 *)ea + nlen + 1 + vlen > end)
> +		    (u8 *)ea->ea_data + nlen + 1 + vlen > end)
> 			return -EINVAL;
> 
> 		switch (vlen) {
> -- 
> 2.53.0
> 
> 
> 
> 

[PATCH 2/2] smb: client: fix OOB reads parsing symlink error response

[Greg Kroah-Hartman]

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: Steve French <sfrench@samba.org>
Cc: Paulo Alcantara <pc@manguebit.org>
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>
Assisted-by: gregkh_clanker_t1000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.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 ed651c946251..b292aa94a593 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;
@@ -128,8 +130,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

Re: [PATCH 2/2] smb: client: fix OOB reads parsing symlink error response

[ChenXiaoSong]

Sashiko reported the following out-of-bounds issue. I have checked and 
confirmed that this indeed causes an OOB access.

When create fails on symlink, `len` in `smb2_check_message()` may be 
smaller than `calc_len`. The function flow is as follows:

```
smb2_check_message()
   // ensure StructureSize2 is 9
   if (... pdu->StructureSize2 != SMB2_ERROR_STRUCTURE_SIZE2_LE ...) // 
false
   smb2_calc_size()
     len = le16_to_cpu(shdr->StructureSize) == 64
     len += le16_to_cpu(pdu->StructureSize2) == 64 + 9
     smb2_get_data_area_len
       if (shdr->StructureSize == 9) // true, return NULL
   calc_len == 64 + 9
   if (len != calc_len) { // true
   /* create failed on symlink */
   if (command == SMB2_CREATE_HE && shdr->Status == 
STATUS_STOPPED_ON_SYMLINK) // true
```



Should we add the following check? Or check it in symlink_data()?

```
-- a/fs/smb/client/smb2misc.c
+++ b/fs/smb/client/smb2misc.c
@@ -241,7 +241,8 @@ smb2_check_message(char *buf, unsigned int pdu_len, 
unsigned int len,
         if (len != calc_len) {
                 /* create failed on symlink */
                 if (command == SMB2_CREATE_HE &&
-                   shdr->Status == STATUS_STOPPED_ON_SYMLINK)
+                   shdr->Status == STATUS_STOPPED_ON_SYMLINK &&
+                   len > calc_len)
                         return 0;
                 /* Windows 7 server returns 24 bytes more */
                 if (calc_len + 24 == len && command == 
SMB2_OPLOCK_BREAK_HE)

```

>> --- 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) {
> Since smb2_check_message() returns success without length validation for
> the symlink error response, is it possible for iov->iov_len to be smaller
> than sizeof(struct smb2_err_rsp)?
> If the buffer only contains the base SMB2 header (64 bytes), does accessing
> err->ErrorContextCount (at offset 66) or err->ByteCount later in this
> function cause an out-of-bounds read?

-- 
ChenXiaoSong <chenxiaosong@kylinos.cn>
Chinese Homepage: chenxiaosong.com
English Homepage: chenxiaosong.com/en

Re: [PATCH 0/2] smb: some potential bugfixes

[Paulo Alcantara]

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

> I spent the time exercising some new fuzzing tools on the ksmbd and smb
> code purely because it's something that is simple to set up and test
> locally with virtual machines, and in doing so, potentially found some
> minor problems for when you have an "untrusted" client.
>
> Here's some fixes for what I happened to notice.  They pass my very
> limited testing here, but please don't trust them at all and verify that
> I'm not just making this all up before accepting them.

Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>

Re: [PATCH 0/2] smb: some potential bugfixes

[Greg Kroah-Hartman]

On Tue, Apr 07, 2026 at 03:52:21PM -0500, Steve French wrote:
> merged into cifs-2.6.git for-next

Thanks!