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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

On 4/2/26 9:21 AM, Sascha Hauer wrote:
> 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>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  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);
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |