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

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

[Shahriyar Jalayeri]

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 bounds the READ length by NFS_READ_SIZE. Patch 2 rejects a negative
readlink length and measures its bound from the symlink data. 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 <shahriyar@byteray.co.uk>
---
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: reject a negative or oversized NFS read length
      net: nfs: reject a negative or oversized readlink length
      test: dm: nfs: add regression tests for the NFS reply-length checks

 configs/sandbox_defconfig |  1 +
 net/nfs-common.c          |  9 ++++-
 test/dm/Makefile          |  1 +
 test/dm/nfs.c             | 90 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 100 insertions(+), 1 deletion(-)
---
base-commit: baa64b2f892890f00a377eac4a3e685472bb56b5
change-id: 20260811-nfs-oob-fix-31c433a22c91

Best regards,
--  
Shahriyar Jalayeri <shahriyar@byteray.co.uk>

[PATCH v2 1/3] net: nfs: reject a negative or oversized NFS read length

[Shahriyar Jalayeri]

nfs_read_reply() stores the server-supplied read length in a signed int
rlen and checks it with:

	if (((uchar *)&rpc_pkt.u.reply.data[0] - (uchar *)&rpc_pkt + rlen) > len)
		return -9999;

On an LP64 target the pointer subtraction is a 64-bit ptrdiff_t, so a
length with the top bit set makes rlen negative, the sum stays negative
and the check passes. store_block() then takes rlen as an unsigned int,
so 0x80000000 becomes a ~2 GB length. memcpy() reads past the 1152-byte
rpc_pkt stack buffer and writes past image_load_addr.

A large positive rlen is also unsafe. The check bounds it by the packet
length rather than by rpc_pkt, so on the NFSv3 path (data_ptr at offset
128) an rlen up to 1128 still reads past the end of rpc_pkt.

Bound the length by NFS_READ_SIZE, the amount a read ever requests,
before it is used. Both the classic and the lwIP NFS clients reach this
through nfs_pkt_recv(), so the single check covers both.

Fixes: aa207cf3a6d6 ("CVE-2019-14194/CVE-2019-14198: nfs: fix unbounded memcpy with a failed length check at nfs_read_reply")
Signed-off-by: Shahriyar Jalayeri <shahriyar@byteray.co.uk>
---
 net/nfs-common.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/nfs-common.c b/net/nfs-common.c
index 72d8fd823e3..020b0185ad1 100644
--- a/net/nfs-common.c
+++ b/net/nfs-common.c
@@ -738,6 +738,10 @@ static int nfs_read_reply(uchar *pkt, unsigned int len)
 			&rpc_pkt.u.reply.data[4 + nfsv3_data_offset];
 	}
 
+	/* reject a negative or too-large length */
+	if (rlen < 0 || rlen > NFS_READ_SIZE)
+		return -9999;
+
 	if (((uchar *)&rpc_pkt.u.reply.data[0] - (uchar *)&rpc_pkt + rlen) > len)
 		return -9999;
 

-- 
2.43.0

[PATCH v2 2/3] net: nfs: reject a negative or oversized readlink length

[Shahriyar Jalayeri]

nfs_readlink_reply() reads the symlink length from the server into a signed
int rlen and bounds it with

	if (((uchar *)&rpc_pkt.u.reply.data[0] - (uchar *)&rpc_pkt + rlen) > len)
		return -NFS_RPC_DROP;

This misses two cases. A negative rlen makes the sum smaller than len, so the
check passes; rlen is then used as an unsigned size_t in memcpy(), and in the
relative-symlink branch pathlen + rlen also stays below the buffer size, so a
length of -1 drives a memcpy() off nfs_path_buff. The bound is also measured
from the reply header rather than from the symlink data, which begins a few
words later, so a large positive rlen reads past the end of the received
reply.

A malicious server answers the READ with an ISDIR status to move the client
into the readlink state, then returns such a reply.

Reject a negative length and measure the bound from the symlink data.

Fixes: cf3a4f1e86ec ("CVE-2019-14195: nfs: fix unbounded memcpy with unvalidated length at nfs_readlink_reply")
Signed-off-by: Shahriyar Jalayeri <shahriyar@byteray.co.uk>
---
 net/nfs-common.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/nfs-common.c b/net/nfs-common.c
index 020b0185ad1..6a536cd5229 100644
--- a/net/nfs-common.c
+++ b/net/nfs-common.c
@@ -666,7 +666,10 @@ static int nfs_readlink_reply(uchar *pkt, unsigned int len)
 	/* new path length */
 	rlen = ntohl(rpc_pkt.u.reply.data[1 + nfsv3_data_offset]);
 
-	if (((uchar *)&rpc_pkt.u.reply.data[0] - (uchar *)&rpc_pkt + rlen) > len)
+	/* reject a negative length or one that runs past the packet */
+	if (rlen < 0 ||
+	    ((uchar *)&rpc_pkt.u.reply.data[2 + nfsv3_data_offset] -
+	     (uchar *)&rpc_pkt + rlen) > len)
 		return -NFS_RPC_DROP;
 
 	if (*((char *)&rpc_pkt.u.reply.data[2 + nfsv3_data_offset]) != '/') {

-- 
2.43.0

[PATCH v2 3/3] test: dm: nfs: add regression tests for the NFS reply-length checks

[Shahriyar Jalayeri]

Add DM tests that feed nfs_pkt_recv() crafted NFSv3 replies with a read and
a readlink request outstanding. The READ reply carries a count with the top
bit set; the READLINK reply carries a length of -1 that slips past the
destination bound as pathlen - 1. Either would drive a memcpy() out of the
reply buffer; the tests assert that nothing is stored and the path buffer is
left untouched.

Enable CONFIG_CMD_NFS in sandbox_defconfig so the NFS client and these tests
are built and run under sandbox.

Signed-off-by: Shahriyar Jalayeri <shahriyar@byteray.co.uk>
---
 configs/sandbox_defconfig |  1 +
 test/dm/Makefile          |  1 +
 test/dm/nfs.c             | 90 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 92 insertions(+)

diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 79f46317e45..ca73080b06d 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -123,6 +123,7 @@ CONFIG_CMD_LINK_LOCAL=y
 CONFIG_IPV6_ROUTER_DISCOVERY=y
 CONFIG_CMD_ETHSW=y
 CONFIG_CMD_DNS=y
+CONFIG_CMD_NFS=y
 CONFIG_CMD_SNTP=y
 CONFIG_CMD_2048=y
 CONFIG_CMD_BMP=y
diff --git a/test/dm/Makefile b/test/dm/Makefile
index fb3e6a7008f..cc51fd33079 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -78,6 +78,7 @@ obj-$(CONFIG_MUX_MMIO) += mux-mmio.o
 obj-y += fdtdec.o
 obj-$(CONFIG_MTD_RAW_NAND) += nand.o
 obj-$(CONFIG_IP_DEFRAG) += net_defrag.o
+obj-$(CONFIG_CMD_NFS) += nfs.o
 obj-$(CONFIG_UT_DM) += nop.o
 obj-y += ofnode.o
 obj-y += ofread.o
diff --git a/test/dm/nfs.c b/test/dm/nfs.c
new file mode 100644
index 00000000000..c7ab1e912b8
--- /dev/null
+++ b/test/dm/nfs.c
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Regression tests for the NFS reply-length checks.
+ */
+
+#include <net.h>
+#include <string.h>
+#include <test/ut.h>
+#include <dm/test.h>
+#include "../../net/nfs-common.h"
+
+static int dm_test_nfs_read_oob(struct unit_test_state *uts)
+{
+	int saved_state = nfs_state;
+	unsigned long saved_id = rpc_id;
+	int saved_offset = nfs_offset;
+	enum nfs_version saved_version = choosen_nfs_version;
+	u32 saved_size = net_boot_file_size;
+	struct rpc_t reply;
+
+	/* Pretend a READ request is outstanding (NFSv3). */
+	choosen_nfs_version = NFS_V3;
+	nfs_state = STATE_READ_REQ;
+	nfs_offset = 0;
+	rpc_id = 0x11223344;
+	net_boot_file_size = 0;
+
+	/* Accepted reply, matching xid, READ status OK, no attributes, then a
+	 * count with the top bit set.
+	 */
+	memset(&reply, 0, sizeof(reply));
+	reply.u.reply.id = htonl((u32)rpc_id);
+	reply.u.reply.data[0] = 0;			/* nfsstat3: OK */
+	reply.u.reply.data[1] = 0;			/* attributes_follow: no */
+	reply.u.reply.data[2] = htonl(0x80000000);	/* count */
+
+	nfs_pkt_recv((uchar *)&reply.u.reply, sizeof(reply.u.reply));
+
+	/* Rejected: nothing stored. */
+	ut_asserteq(0, net_boot_file_size);
+
+	nfs_state = saved_state;
+	rpc_id = saved_id;
+	nfs_offset = saved_offset;
+	choosen_nfs_version = saved_version;
+	net_boot_file_size = saved_size;
+
+	return 0;
+}
+DM_TEST(dm_test_nfs_read_oob, 0);
+
+static int dm_test_nfs_readlink_oob(struct unit_test_state *uts)
+{
+	int saved_state = nfs_state;
+	unsigned long saved_id = rpc_id;
+	enum nfs_version saved_version = choosen_nfs_version;
+	char *saved_path = nfs_path;
+	struct rpc_t reply;
+
+	/* Pretend a READLINK request is outstanding (NFSv3). */
+	choosen_nfs_version = NFS_V3;
+	nfs_state = STATE_READLINK_REQ;
+	rpc_id = 0x11223344;
+	nfs_path = nfs_path_buff;
+	strcpy(nfs_path_buff, "dir");
+
+	/* Accepted reply, matching xid, READLINK status OK, no attributes, a
+	 * length of -1 that slips past the destination bound as pathlen - 1,
+	 * then a relative (non-'/') target.
+	 */
+	memset(&reply, 0, sizeof(reply));
+	reply.u.reply.id = htonl((u32)rpc_id);
+	reply.u.reply.data[0] = 0;			/* nfsstat3: OK */
+	reply.u.reply.data[1] = 0;			/* attributes_follow: no */
+	reply.u.reply.data[2] = htonl(0xffffffff);	/* symlink length -1 */
+	reply.u.reply.data[3] = htonl(0x61616161);	/* target, not '/' */
+
+	nfs_pkt_recv((uchar *)&reply.u.reply, sizeof(reply.u.reply));
+
+	/* Rejected: the path buffer is untouched. */
+	ut_asserteq_str("dir", nfs_path_buff);
+
+	nfs_state = saved_state;
+	rpc_id = saved_id;
+	choosen_nfs_version = saved_version;
+	nfs_path = saved_path;
+
+	return 0;
+}
+DM_TEST(dm_test_nfs_readlink_oob, 0);

-- 
2.43.0

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

[Jerome Forissier]

Hi Shahriyar,

On 13/08/2026 17:48, 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 bounds the READ length by NFS_READ_SIZE. Patch 2 rejects a negative
> readlink length and measures its bound from the symlink data. Patch 3
> enables CONFIG_CMD_NFS in the sandbox config and adds DM regression tests
> that drive nfs_pkt_recv() with crafted replies.


I think the rlen < 0 check in patches 1 and 2 is awkward and not fully robust.
I suggest making rlen a u32 instead.

- For nfs_read_reply() in patch 1:

  u32 rlen;
  size_t data_offset;

  rlen = ntohl(rpc_pkt.u.reply.data[3 + nfsv3_data_offset]);
  if (rlen > NFS_READ_SIZE)
          return -9999;

  data_offset = data_ptr - (uchar *)&rpc_pkt;
  if (data_offset > len || rlen > len - data_offset)
          return -9999;

- For nfs_readlink_reply() in patch 2:

  u32 rlen;
  size_t data_offset;

  rlen = ntohl(rpc_pkt.u.reply.data[1 + nfsv3_data_offset]);
  data_offset = (uchar *)&rpc_pkt.u.reply.data[2 + nfsv3_data_offset] -
                (uchar *)&rpc_pkt;

  if (data_offset > len || rlen > len - data_offset)
           return -NFS_RPC_DROP;

In addition, nfs_readlink_reply() has this check:

  if (pathlen + rlen >= sizeof(nfs_path_buff))

IMO pathlen should be size_t and the test replaced with:

  if (pathlen >= sizeof(nfs_path_buff) ||
      rlen >= sizeof(nfs_path_buff) - pathlen)
          return -NFS_RPC_DROP;

What do you think?

Thanks,
--
Jerome

>
> A reproducer is available on request.
>
> Signed-off-by: Shahriyar Jalayeri <shahriyar@byteray.co.uk>
> ---
> 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: reject a negative or oversized NFS read length
>       net: nfs: reject a negative or oversized readlink length
>       test: dm: nfs: add regression tests for the NFS reply-length checks
>
>  configs/sandbox_defconfig |  1 +
>  net/nfs-common.c          |  9 ++++-
>  test/dm/Makefile          |  1 +
>  test/dm/nfs.c             | 90 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 100 insertions(+), 1 deletion(-)
> ---
> base-commit: baa64b2f892890f00a377eac4a3e685472bb56b5
> change-id: 20260811-nfs-oob-fix-31c433a22c91
>
> Best regards,
> --
> Shahriyar Jalayeri <shahriyar@byteray.co.uk>
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.