[PATCH 0/4] tftp: fix buffer overflows

[PATCH 0/4] tftp: fix buffer overflows

[Sascha Hauer]

This adds more checks on incoming network data to prevent buffer
overflows.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Sascha Hauer (4):
      fs: tftp: prevent packet buffer overflow from long filenames
      fs: tftp: fix OACK option parsing bounds check
      fs: tftp: reject OACK with blocksize of zero
      fs: tftp: use bounded format for TFTP error message debug print

 fs/tftp.c | 45 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 10 deletions(-)
---
base-commit: 0933e8f2ebf0d91dfcf177a4e4292b02921a53f1
change-id: 20260402-net-tftp-buffer-overflows-6b839ef57261

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

[PATCH 1/4] fs: tftp: prevent packet buffer overflow from long filenames

[Sascha Hauer]

tftp_send() constructs RRQ/WRQ packets by sprintf'ing the filename and
TFTP options directly into the packet buffer with no bounds checking.
The packet buffer is PKTSIZE (1536) bytes with 42 bytes of headers,
leaving 1494 bytes of UDP payload. A filename longer than ~1418 bytes
overflows the packet buffer, corrupting adjacent heap memory.

Replace sprintf with snprintf, tracking available buffer space and
returning -ENAMETOOLONG if the packet would exceed the buffer.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 fs/tftp.c | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/fs/tftp.c b/fs/tftp.c
index a454306b4b..1b15bc18e7 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -79,6 +79,10 @@
 /* allocate this number of blocks more than needed in the fifo */
 #define TFTP_EXTRA_BLOCKS	2
 
+/* Maximum UDP payload that fits in a PKTSIZE packet buffer */
+#define TFTP_MAX_UDP_PAYLOAD	(PKTSIZE - ETHER_HDR_SIZE - \
+				 sizeof(struct iphdr) - sizeof(struct udphdr))
+
 /* marker for an emtpy 'tftp_cache' */
 #define TFTP_CACHE_NO_ID	(-1)
 
@@ -218,7 +222,9 @@ static int tftp_send(struct file_priv *priv)
 
 	switch (priv->state) {
 	case STATE_RRQ:
-	case STATE_WRQ:
+	case STATE_WRQ: {
+		int room, n;
+
 		if (priv->push || priv->is_getattr)
 			/* atm, windowsize is supported only for RRQ and there
 			   is no need to request a full window when we are
@@ -235,7 +241,9 @@ static int tftp_send(struct file_priv *priv)
 		else
 			*s++ = htons(TFTP_WRQ);
 		pkt = (unsigned char *)s;
-		pkt += sprintf((unsigned char *)pkt,
+		room = TFTP_MAX_UDP_PAYLOAD - (pkt - xp);
+
+		n = snprintf(pkt, room,
 				"%s%c"
 				"octet%c"
 				"timeout%c"
@@ -250,24 +258,38 @@ static int tftp_send(struct file_priv *priv)
 				/* use only a minimal blksize for getattr
 				   operations, */
 				priv->is_getattr ? TFTP_BLOCK_SIZE : TFTP_MTU_SIZE);
-		pkt++;
+		if (n >= room)
+			return -ENAMETOOLONG;
+		pkt += n + 1;
+		room -= n + 1;
 
-		if (!priv->push)
+		if (!priv->push) {
 			/* we do not know the filesize in WRQ requests and
 			   'priv->filesize' will always be zero */
-			pkt += sprintf((unsigned char *)pkt,
+			n = snprintf(pkt, room,
 				       "tsize%c%lld%c",
 				       '\0', priv->filesize,
 				       '\0');
+			if (n >= room)
+				return -ENAMETOOLONG;
+			pkt += n;
+			room -= n;
+		}
 
-		if (window_size > 1)
-			pkt += sprintf((unsigned char *)pkt,
+		if (window_size > 1) {
+			n = snprintf(pkt, room,
 				       "windowsize%c%u%c",
 				       '\0', window_size,
 				       '\0');
+			if (n >= room)
+				return -ENAMETOOLONG;
+			pkt += n;
+			room -= n;
+		}
 
 		len = pkt - xp;
 		break;
+	}
 
 	case STATE_RDATA:
 		xp = pkt;

-- 
2.47.3

[PATCH 2/4] fs: tftp: fix OACK option parsing bounds check

[Sascha Hauer]

The bounds check in tftp_parse_oack() uses 'val > s + len' to detect
when the value pointer exceeds the packet. Since 's' advances through
the buffer while 'len' stays constant, 's + len' always points past
'pkt + len', making the check always false — it is dead code.

The forced null at pkt[len - 1] provides partial protection, but when
the last option key ends exactly at pkt[len - 1], val equals pkt + len
and the subsequent strlen(val) reads past the packet boundary into
uninitialized packet buffer data.

Fix by checking 'val >= pkt + len' instead, which correctly bounds val
against the actual end of the packet data. Using >= because when val
equals pkt + len there is no room for a value string.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 fs/tftp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/tftp.c b/fs/tftp.c
index 1b15bc18e7..03e9d552aa 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -370,7 +370,7 @@ static int tftp_parse_oack(struct file_priv *priv, unsigned char *pkt, int len)
 	while (s < pkt + len) {
 		opt = s;
 		val = s + strlen(s) + 1;
-		if (val > s + len)
+		if (val >= pkt + len)
 			break;
 		if (!strcmp(opt, "tsize"))
 			priv->filesize = simple_strtoull(val, NULL, 10);

-- 
2.47.3

[PATCH 3/4] fs: tftp: reject OACK with blocksize of zero

[Sascha Hauer]

tftp_parse_oack() validates that windowsize is non-zero but does not
check blocksize. A malicious TFTP server sending 'blksize\0000\0' in
the OACK sets priv->blocksize to 0, which causes:

  - kfifo_alloc(0 * ...) allocating a zero-sized fifo
  - tftp_write() entering an infinite loop on
    'while (kfifo_len(fifo) >= 0)', spinning on kfifo_get with size 0
  - tftp_put_data() rejecting all data packets as oversized

Add blocksize == 0 to the existing OACK validation check.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 fs/tftp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/tftp.c b/fs/tftp.c
index 03e9d552aa..6dd4829184 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -382,7 +382,8 @@ static int tftp_parse_oack(struct file_priv *priv, unsigned char *pkt, int len)
 		s = val + strlen(val) + 1;
 	}
 
-	if (priv->blocksize > TFTP_MTU_SIZE ||
+	if (priv->blocksize == 0 ||
+	    priv->blocksize > TFTP_MTU_SIZE ||
 	    priv->windowsize > TFTP_MAX_WINDOW_SIZE ||
 	    priv->windowsize == 0) {
 		pr_warn("tftp: invalid oack response\n");

-- 
2.47.3

[PATCH 4/4] fs: tftp: use bounded format for TFTP error message debug print

[Sascha Hauer]

The pr_debug for TFTP_ERROR uses %s to print the server's error
message, but the message may not be null-terminated. This causes
pr_debug to read past the packet data into uninitialized buffer
memory.

Use %.*s with the remaining packet length to bound the read. This
is only relevant in debug builds but is still worth fixing for
correctness.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 fs/tftp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/tftp.c b/fs/tftp.c
index 6dd4829184..8f01754fc0 100644
--- a/fs/tftp.c
+++ b/fs/tftp.c
@@ -631,7 +631,9 @@ static void tftp_recv(struct file_priv *priv,
 		break;
 
 	case TFTP_ERROR:
-		pr_debug("error: '%s' (%d)\n", pkt + 2, ntohs(*(uint16_t *)pkt));
+		pr_debug("error: '%.*s' (%d)\n",
+			 len > 2 ? len - 2 : 0, pkt + 2,
+			 ntohs(*(uint16_t *)pkt));
 		switch (ntohs(*(uint16_t *)pkt)) {
 		case 1:
 			priv->err = -ENOENT;

-- 
2.47.3