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

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

[Jeremy Erazo]

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@gmail.com>
---
 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;

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@gmail.com> 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@gmail.com>
> ---
>  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

[PATCH] smb: client: detect short folioq copy in cifs_copy_folioq_to_iter()

[Jeremy Erazo]

cifs_copy_folioq_to_iter() copies a requested number of bytes from
a folio queue into the destination iterator.  Since the encrypted
SMB2 READ path was changed to pass the server-declared payload
length (data_len) instead of the larger folioq buffer length, the
caller can ask for fewer bytes than the folio queue holds.

In that case the helper continues walking the remaining folios after
data_size has reached zero and calls copy_folio_to_iter() with
len = 0, which is unnecessary work.

The helper also returns 0 (success) when the folio queue is
exhausted before data_size bytes have been copied.  The caller has
no way to distinguish that from a full copy and the reported
transfer count ends up larger than the amount of data placed in the
iterator.

Add an early exit when data_size reaches zero, and return an error
when the folio queue is exhausted before all requested bytes have
been copied.

Signed-off-by: Jeremy Erazo <mendozayt13@gmail.com>
---
 fs/smb/client/smb2ops.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index ee8370026..1dd06c31f 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -4696,9 +4696,15 @@ cifs_copy_folioq_to_iter(struct folio_queue *folioq, size_t data_size,
 {
 	for (; folioq; folioq = folioq->next) {
 		for (int s = 0; s < folioq_count(folioq); s++) {
-			struct folio *folio = folioq_folio(folioq, s);
-			size_t fsize = folio_size(folio);
-			size_t n, len = umin(fsize - skip, data_size);
+			struct folio *folio;
+			size_t fsize, n, len;
+
+			if (data_size == 0)
+				return 0;
+
+			folio = folioq_folio(folioq, s);
+			fsize = folio_size(folio);
+			len = umin(fsize - skip, data_size);
 
 			n = copy_folio_to_iter(folio, skip, len, iter);
 			if (n != len) {
@@ -4711,6 +4717,12 @@ cifs_copy_folioq_to_iter(struct folio_queue *folioq, size_t data_size,
 		}
 	}
 
+	if (data_size != 0) {
+		cifs_dbg(VFS, "%s: short copy, %zu bytes missing\n",
+			 __func__, data_size);
+		return smb_EIO2(smb_eio_trace_rx_copy_to_iter, 0, data_size);
+	}
+
 	return 0;
 }
 
-- 
2.53.0