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
