Re: [PATCH] smb: client: use data_len for SMB2 READ encrypted folioq copy

[Steve French]

Don't remember if I had forwarded the sashiko AI review of the patch to you

https://sashiko.dev/#/patchset/20260515193141.542623-1-mendozayt13%40gmail.com

Let me know if any changes needed

On Fri, May 15, 2026 at 2:31 PM Jeremy Erazo <mendozayt13@xxxxxxxxx> wrote:
>
> In handle_read_data() the encrypted/folioq branch
> (buf_len <= data_offset, reached via receive_encrypted_read for
> transform PDUs > CIFSMaxBufSize + MAX_HEADER_SIZE) copies the READ
> payload using buffer_len rather than data_len:
>
>         rdata->result = cifs_copy_folioq_to_iter(buffer, buffer_len,
>                                                  cur_off,
>                                                  &rdata->subreq.io_iter);
>         ...
>         rdata->got_bytes = buffer_len;
>
> buffer_len comes from the SMB3 transform header OriginalMessageSize
> field (OriginalMessageSize - read_rsp_size); it represents the size
> of the decrypted message after the SMB2 header.  data_len comes from
> the SMB2 READ response DataLength field; it represents the actual
> READ payload size and may be smaller than buffer_len when the
> decrypted message contains padding or other trailing bytes after the
> READ payload.  The existing check `data_len > buffer_len - pad_len`
> only enforces an upper bound, so a server that emits
> OriginalMessageSize larger than read_rsp_size + pad_len + data_len
> passes the check and the kernel copies buffer_len bytes per response,
> ignoring the server-asserted DataLength.
>
> Two observable failures with a crafted server (DataLength=4,
> buffer_len=20000):
>
>   - the kernel returns 20000 bytes per sub-request to userspace and
>     sets got_bytes = buffer_len, even though the response claimed
>     only 4 bytes of payload;
>
>   - on a partial netfs sub-request whose iterator is sized to
>     data_len, the over-large copy_folio_to_iter() short-reads,
>     cifs_copy_folioq_to_iter() returns -EIO via the n != len path,
>     and the entire netfs read collapses to -EIO even though the
>     leading sub-requests succeeded.
>
> Use data_len for the copy length and for got_bytes so the kernel
> honours the server-asserted READ payload size.  For well-formed
> servers (where buffer_len == pad_len + data_len) the change is
> behaviour-equivalent.
>
> Signed-off-by: Jeremy Erazo <mendozayt13@xxxxxxxxx>
> ---
>  fs/smb/client/smb2ops.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> --- a/fs/smb/client/smb2ops.c
> +++ b/fs/smb/client/smb2ops.c
> @@ -4825,7 +4825,7 @@ handle_read_data(struct TCP_Server_Info *server, struct mid_q_entry *mid,
>                 }
>
>                 /* Copy the data to the output I/O iterator. */
> -               rdata->result = cifs_copy_folioq_to_iter(buffer, buffer_len,
> +               rdata->result = cifs_copy_folioq_to_iter(buffer, data_len,
>                                                          cur_off, &rdata->subreq.io_iter);
>                 if (rdata->result != 0) {
>                         if (is_offloaded)
> @@ -4834,5 +4834,5 @@ handle_read_data(struct TCP_Server_Info *server, struct mid_q_entry *mid,
>                                 dequeue_mid(server, mid, rdata->result);
>                         return 0;
>                 }
> -               rdata->got_bytes = buffer_len;
> +               rdata->got_bytes = data_len;
>


-- 
Thanks,

Steve