[PATCH 0/4] Bounds/overflow hardening in NFS, FIT, TFTP and ext4

[PATCH 0/4] Bounds/overflow hardening in NFS, FIT, TFTP and ext4

[Sriram Sriram]

This series collects four independent robustness fixes to network, boot
image and filesystem code that handle attacker-influenced or on-disk
input. Each was found by auditing length handling around memcpy(),
integer arithmetic on packet/image data, and loop-counter widths.

  1. net: nfs: the NFS client copies received UDP payloads into a
     stack-allocated struct rpc_t with memcpy() using the wire length
     without checking it against the destination size, allowing a
     malicious NFS server to overflow the stack buffer. Add a bounds
     check at each copy site.

  2. boot: image-fit: the decompression path computes the output buffer
     size as 'len * 20', which can wrap on 32/64-bit ulong for a large
     image and cause a heap buffer overflow. Reject sizes that would
     overflow before the multiplication.

  3. net: tftp: the OACK option parser uses strcasecmp() on packet data
     that may not be NUL-terminated within the received length, causing
     an out-of-bounds read. Use a bounded strncasecmp() plus an explicit
     terminator check.

  4. fs: ext4: ext4fs_update() walks all block groups with a signed
     16-bit loop counter while fs->no_blkgrp is a uint32_t. A filesystem
     with more than 32767 block groups overflows the counter (undefined
     behaviour) and the bitmap write-back loops fail to terminate
     correctly. Widen the counter to u32.

The fixes are independent and can be applied in any order. Built for
sandbox (net/tftp.o, boot/image-fit.o, fs/ext4/ext4_write.o, and
net/nfs-common.o with CONFIG_CMD_NFS=y) and checked with
scripts/checkpatch.pl.

Daniel Munic (1):
  fs: ext4: widen ext4fs_update() block-group loop counter

Drew Kluemke (3):
  net: nfs: add bounds checks on memcpy into stack-allocated rpc_pkt
  boot: image-fit: add overflow guard for FIT decompression buffer
  net: tftp: use bounded string compare for OACK option parsing

 boot/image-fit.c     | 10 +++++++++-
 fs/ext4/ext4_write.c |  2 +-
 net/nfs-common.c     | 10 ++++++++++
 net/tftp.c           | 13 +++++++++----
 4 files changed, 29 insertions(+), 6 deletions(-)

-- 
2.49.0

[PATCH 1/4] net: nfs: add bounds checks on memcpy into stack-allocated rpc_pkt

[Sriram Sriram]

From: Drew Kluemke <ankluemk@microsoft.com>

Multiple NFS reply handlers (rpc_lookup_reply, nfs_mount_reply,
nfs_umountall_reply, nfs_lookup_reply, nfs_readlink_reply) copy network
data into a stack-allocated struct rpc_t without verifying that the UDP
payload length fits within the buffer. A malicious or malformed NFS
server response with len > sizeof(rpc_pkt) overwrites the stack frame.

Add a bounds check before each memcpy to drop oversized packets.

Signed-off-by: Drew Kluemke <ankluemk@microsoft.com>
Signed-off-by: Sriram Sriram <sriramsriram@linux.microsoft.com>
---
 net/nfs-common.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/net/nfs-common.c b/net/nfs-common.c
index 637fcfd9bb8..226e3fe5798 100644
--- a/net/nfs-common.c
+++ b/net/nfs-common.c
@@ -503,6 +503,8 @@ static int rpc_lookup_reply(int prog, uchar *pkt, unsigned int len)
 {
 	struct rpc_t rpc_pkt;
 
+	if (len > sizeof(rpc_pkt.u.data))
+		return -NFS_RPC_DROP;
 	memcpy(&rpc_pkt.u.data[0], pkt, len);
 
 	if (ntohl(rpc_pkt.u.reply.id) > rpc_id)
@@ -532,6 +534,8 @@ static int nfs_mount_reply(uchar *pkt, unsigned int len)
 	struct rpc_t rpc_pkt;
 	int ret;
 
+	if (len > sizeof(rpc_pkt.u.data))
+		return -NFS_RPC_DROP;
 	memcpy(&rpc_pkt.u.data[0], pkt, len);
 
 	if (ntohl(rpc_pkt.u.reply.id) > rpc_id)
@@ -561,6 +565,8 @@ static int nfs_umountall_reply(uchar *pkt, unsigned int len)
 {
 	struct rpc_t rpc_pkt;
 
+	if (len > sizeof(rpc_pkt.u.data))
+		return -NFS_RPC_DROP;
 	memcpy(&rpc_pkt.u.data[0], pkt, len);
 
 	if (ntohl(rpc_pkt.u.reply.id) > rpc_id)
@@ -584,6 +590,8 @@ static int nfs_lookup_reply(uchar *pkt, unsigned int len)
 	struct rpc_t rpc_pkt;
 	int ret;
 
+	if (len > sizeof(rpc_pkt.u.data))
+		return -NFS_RPC_DROP;
 	memcpy(&rpc_pkt.u.data[0], pkt, len);
 
 	if (ntohl(rpc_pkt.u.reply.id) > rpc_id)
@@ -645,6 +653,8 @@ static int nfs_readlink_reply(uchar *pkt, unsigned int len)
 	int rlen;
 	int nfsv3_data_offset = 0;
 
+	if (len > sizeof(rpc_pkt))
+		return -NFS_RPC_DROP;
 	memcpy((unsigned char *)&rpc_pkt, pkt, len);
 
 	if (ntohl(rpc_pkt.u.reply.id) > rpc_id)
-- 
2.49.0

[PATCH 2/4] boot: image-fit: add overflow guard for FIT decompression buffer

[Sriram Sriram]

From: Drew Kluemke <ankluemk@microsoft.com>

Add an overflow check before the 'len * 20' multiplication used to
compute the maximum decompression buffer size. On platforms where len
exceeds ULONG_MAX / 20 the multiplication wraps to a small value,
leading malloc to allocate a tiny buffer while decompression writes the
full stream -- a heap buffer overflow.

Guard against this by returning -ENOEXEC when len exceeds the safe
threshold.

Signed-off-by: Drew Kluemke <ankluemk@microsoft.com>
Signed-off-by: Sriram Sriram <sriramsriram@linux.microsoft.com>
---
 boot/image-fit.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/boot/image-fit.c b/boot/image-fit.c
index 26e9323da06..9770435f939 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -2355,9 +2355,17 @@ int fit_image_load(struct bootm_headers *images, ulong addr,
 	    !(image_type == IH_TYPE_KERNEL ||
 	      image_type == IH_TYPE_KERNEL_NOLOAD ||
 	      image_type == IH_TYPE_RAMDISK)) {
-		ulong max_decomp_len = len * 20;
+		ulong max_decomp_len;
 
 		log_debug("decompressing image\n");
+
+		if (len > ULONG_MAX / 20) {
+			printf("Error: %s image too large for decompression (0x%lx)\n",
+			       prop_name, len);
+			return -ENOEXEC;
+		}
+		max_decomp_len = len * 20;
+
 		if (load == data) {
 			loadbuf = aligned_alloc(8, max_decomp_len);
 			load = map_to_sysmem(loadbuf);
-- 
2.49.0

Re: [PATCH 2/4] boot: image-fit: add overflow guard for FIT decompression buffer

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 595 bytes --]

On Wed, Sep 09, 2026 at 12:20:14PM -0700, Sriram Sriram wrote:

> From: Drew Kluemke <ankluemk@microsoft.com>
> 
> Add an overflow check before the 'len * 20' multiplication used to
> compute the maximum decompression buffer size. On platforms where len
> exceeds ULONG_MAX / 20 the multiplication wraps to a small value,
> leading malloc to allocate a tiny buffer while decompression writes the
> full stream -- a heap buffer overflow.

OK, so lets assume an overflow, or just an invalid max_decomp_len,
shouldn't image_decomp handle this case and fail and we catch it?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 3/4] net: tftp: use bounded string compare for OACK option parsing

[Sriram Sriram]

From: Drew Kluemke <ankluemk@microsoft.com>

The TFTP OACK handler matches option names in the received packet with
strcasecmp(), which scans until a NUL byte. A malformed or truncated
OACK packet may not contain a NUL within the received length, causing
an out-of-bounds read past the packet buffer.

Replace each strcasecmp() with a strncasecmp() bounded to the option
name length plus an explicit check that the name is NUL-terminated at
the expected position. For the "windowsize" option, whose name and
value extend beyond the loop's "i + 8 < len" guard, add an explicit
"i + 11 < len" bound before the comparison.

Signed-off-by: Drew Kluemke <ankluemk@microsoft.com>
Signed-off-by: Sriram Sriram <sriramsriram@linux.microsoft.com>
---
 net/tftp.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/tftp.c b/net/tftp.c
index beb9d08f5a2..35693e2a93e 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -544,7 +544,8 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 		 * something like "len-8" may give a *huge* number
 		 */
 		for (i = 0; i+8 < len; i++) {
-			if (strcasecmp((char *)pkt + i, "blksize") == 0) {
+			if (!strncasecmp((char *)pkt + i, "blksize", 7) &&
+			    pkt[i + 7] == '\0') {
 				tftp_block_size = (unsigned short)
 					dectoul((char *)pkt + i + 8, NULL);
 				debug("Blocksize oack: %s, %d\n",
@@ -555,7 +556,8 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 					tftp_state = STATE_INVALID_OPTION;
 				}
 			}
-			if (strcasecmp((char *)pkt + i, "timeout") == 0) {
+			if (!strncasecmp((char *)pkt + i, "timeout", 7) &&
+			    pkt[i + 7] == '\0') {
 				timeout_val_rcvd = (unsigned short)
 					dectoul((char *)pkt + i + 8, NULL);
 				debug("Timeout oack: %s, %d\n",
@@ -567,14 +569,17 @@ static void tftp_handler(uchar *pkt, unsigned dest, struct in_addr sip,
 				}
 			}
 #ifdef CONFIG_TFTP_TSIZE
-			if (strcasecmp((char *)pkt + i, "tsize") == 0) {
+			if (!strncasecmp((char *)pkt + i, "tsize", 5) &&
+			    pkt[i + 5] == '\0') {
 				tftp_tsize = dectoul((char *)pkt + i + 6,
 						     NULL);
 				debug("size = %s, %u\n",
 				      (char *)pkt + i + 6, tftp_tsize);
 			}
 #endif
-			if (strcasecmp((char *)pkt + i,  "windowsize") == 0) {
+			if (i + 11 < len &&
+			    !strncasecmp((char *)pkt + i, "windowsize", 10) &&
+			    pkt[i + 10] == '\0') {
 				tftp_windowsize =
 					dectoul((char *)pkt + i + 11, NULL);
 				debug("windowsize = %s, %d\n",
-- 
2.49.0

Re: [PATCH 3/4] net: tftp: use bounded string compare for OACK option parsing

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1227 bytes --]

On Wed, Sep 09, 2026 at 12:20:15PM -0700, Sriram Sriram wrote:

> From: Drew Kluemke <ankluemk@microsoft.com>
> 
> The TFTP OACK handler matches option names in the received packet with
> strcasecmp(), which scans until a NUL byte. A malformed or truncated
> OACK packet may not contain a NUL within the received length, causing
> an out-of-bounds read past the packet buffer.
> 
> Replace each strcasecmp() with a strncasecmp() bounded to the option
> name length plus an explicit check that the name is NUL-terminated at
> the expected position. For the "windowsize" option, whose name and
> value extend beyond the loop's "i + 8 < len" guard, add an explicit
> "i + 11 < len" bound before the comparison.
> 
> Signed-off-by: Drew Kluemke <ankluemk@microsoft.com>
> Signed-off-by: Sriram Sriram <sriramsriram@linux.microsoft.com>
> ---
>  net/tftp.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)

Does the spec/RFC ensure that we're being given a NULL terminated string
here? It certainly makes sense to be using strncasecmp here, but since
we're a size constrained bootloader, we do need to find the right
balance between defense-in-depth and potential exposure.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 4/4] fs: ext4: widen ext4fs_update() block-group loop counter

[Sriram Sriram]

From: Daniel Munic <v-dmunic@microsoft.com>

ext4fs_update() iterates over all block groups with a signed 16-bit
loop counter:

	short i;
	...
	for (i = 0; i < fs->no_blkgrp; i++)

fs->no_blkgrp is a uint32_t. On a filesystem with more than 32767
block groups the counter cannot represent every index: incrementing
past SHRT_MAX is signed overflow (undefined behaviour) and the
comparison against the unsigned no_blkgrp never terminates correctly,
so the bitmap/group-descriptor write-back loops misbehave. The mixed
signed/unsigned comparison is also flagged by static analysis.

Use u32 for the loop counter, matching the width of no_blkgrp.

Signed-off-by: Daniel Munic <v-dmunic@microsoft.com>
Signed-off-by: Sriram Sriram <sriramsriram@linux.microsoft.com>
---
 fs/ext4/ext4_write.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index 1abedcede72..2fba2197c19 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -67,7 +67,7 @@ static inline void ext4fs_bg_free_blocks_inc
 
 static void ext4fs_update(void)
 {
-	short i;
+	u32 i;
 	ext4fs_update_journal();
 	struct ext_filesystem *fs = get_fs();
 	struct ext2_block_group *bgd = NULL;
-- 
2.49.0