Re: [PATCH] smb: client: reject out-of-bounds DataOffset in CIFSSMBRead()

[Namjae Jeon]

On Sat, Aug 29, 2026 at 12:02 AM Diego Oliva <diego@xxxxxxxx> wrote:
>
> The SMB1 synchronous read helper CIFSSMBRead() validates the server's
> DataLength against CIFSMaxBufSize and the caller's count, but never
> validates DataOffset. The copy source is formed as
>
>         &pSMBr->hdr.Protocol + le16_to_cpu(pSMBr->DataOffset)
>
> and memcpy()'d for DataLength bytes with no check that the
> [DataOffset, DataOffset + DataLength) range lies within the response
> actually received from the server.
>
> A malicious or compromised SMB1 server can return a short response
> carrying an in-range DataLength and a large DataOffset, driving the
> source pointer past the end of the response buffer. The memcpy() then
> copies adjacent kernel heap into the caller's read buffer (information
> disclosure), or reads unmapped memory and oopses (denial of service).
> SMB1 is not negotiated by default; reaching this code requires an
> explicit vers=1.0 mount.
>
> Both DataOffset and the received response length recorded in
> rsp_iov.iov_len are relative to the start of the SMB header, so reject
> the response unless DataOffset + DataLength fits within that length,
> using overflow-safe arithmetic, before forming the source pointer.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Bynario AI
> Signed-off-by: Diego Oliva <diego@xxxxxxxx>
> ---
> Note for backporting: this uses the smb_EIO2() tracepoint helper added
> in v6.19 with f80ac7eda1cf5. For older kernels, the call can be
> replaced with a simple return of -EIO.
>
>  fs/smb/client/cifssmb.c | 11 +++++++++--
>  fs/smb/client/trace.h   |  1 +
>  2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c
> index f5aad5f61dce..1a822121a883 100644
> --- a/fs/smb/client/cifssmb.c
> +++ b/fs/smb/client/cifssmb.c
> @@ -1721,6 +1721,7 @@ CIFSSMBRead(const unsigned int xid, struct cifs_io_parms *io_parms,
>                 cifs_dbg(VFS, "Send error in read = %d\n", rc);
>         } else {
>                 int data_length = le16_to_cpu(pSMBr->DataLengthHigh);
Should we make data_length unsigned also ?

> +               __u16 data_offset = le16_to_cpu(pSMBr->DataOffset);