Re: [PATCH v3 0/3] net: nfs: bound server-supplied lengths in READ and READLINK replies

Hi Shahriyar,

On 19/08/2026 18:40, Shahriyar Jalayeri wrote:
> A malicious NFS server can return replies whose 32-bit lengths are crafted
> to defeat the client's bounds checks.
> 
> nfs_read_reply() keeps the READ length in a signed int. On LP64 a value
> with the top bit set is negative, the bounds check passes, and
> store_block() then hands a ~2 GB length to memcpy(), which reads past the
> 1152-byte reply buffer on the stack and writes past image_load_addr.
> 
> nfs_readlink_reply() has the same signed-length flaw. A length of -1 slips
> past the destination bound as pathlen - 1 and drives a memcpy() off
> nfs_path_buff. The bound is also measured from the reply header rather than
> from the symlink data, so a large positive length reads a few bytes past
> the received reply. A server reaches this handler by answering the READ
> with an ISDIR status, which moves the client into the readlink state.
> 
> Both handlers are shared by the classic and lwIP NFS clients through
> nfs_pkt_recv().
> 
> Patch 1 makes the READ length unsigned and bounds it against the received
> packet and NFS_READ_SIZE. Patch 2 does the same for the readlink length
> and its destination bound. Patch 3 enables CONFIG_CMD_NFS in the sandbox
> config and adds DM regression tests that drive nfs_pkt_recv() with crafted
> replies.
> 
> A reproducer is available on request.
> 
> Signed-off-by: Shahriyar Jalayeri <[email protected]>
> ---
> Changes in v3, all following Jerome Forissier's review:
> - nfs_read_reply() and nfs_readlink_reply(): keep the server-supplied
>   length in an unsigned int and bound it against the received packet with
>   the subtraction ordered so it cannot wrap, dropping the explicit
>   negative check.
> - nfs_read_reply(): take the NFSv3 length from the opaque data_size field
>   that prefixes the returned bytes.
> - test: also set the data length field that nfs_read_reply() now reads.
> 
> Changes in v2:
> - Add a fix for the same signed-length flaw in nfs_readlink_reply(),
>   rejecting a negative length and measuring the bound from the symlink
>   data.
> - Enable CONFIG_CMD_NFS in sandbox_defconfig so the regression tests are
>   built and run under sandbox; the v1 test was skipped in CI.
> - Add a readlink regression test alongside the read one.
> 
> ---
> Shahriyar Jalayeri (3):
>       net: nfs: bound the length of an NFS read reply
>       net: nfs: bound the length of an NFS readlink reply
>       test: dm: nfs: add regression tests for the NFS reply-length checks
> 
>  configs/sandbox_defconfig |  1 +
>  net/nfs-common.c          | 32 ++++++++++-------
>  test/dm/Makefile          |  1 +
>  test/dm/nfs.c             | 91 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 113 insertions(+), 12 deletions(-)
> ---
> base-commit: baa64b2f892890f00a377eac4a3e685472bb56b5
> change-id: 20260811-nfs-oob-fix-31c433a22c91
> 
> Best regards,
> --  
> Shahriyar Jalayeri <[email protected]>

Thanks for the update. This looks good. For the series:

Reviewed-by: Jerome Forissier <[email protected]>

Regards,
-- 
Jerome