[PATCH] net: tftp: fix malformed option string parsing

[Jerome Forissier]

On 23/03/2026 12:58, Sverdlin, Alexander (SI B PRO TI EAC CCP) wrote:
> Hi Chencheng,
> 
> On Mon, 2026-03-23 at 12:05 +0100, Chencheng Zhang wrote:
>> When formatting the TFTP options (tsize, blksize, windowsize), the
>> code previously used sprintf() with embedded null bytes (%c). Because
>> sprintf() returns the length excluding the terminating null byte,
>> subsequent sprintf() calls overwrote the required null separators.
> 
> I'm not convinced, the analysis explains the observed effect,
> 
>> This caused the options to be concatenated into a single malformed
>> string (e.g., "tsize0blksize1468") which breaks strict TFTP servers
>                      ^^
> ... as missing 0 byte between 'e' and '0' is not even at the end...
> 
>> like atftpd, causing them to reply with empty packets or drop the
>> connection.
>>
>> Fix this by explicitly using strcpy() for the option names and manually
>> advancing the pointer to ensure the null bytes are preserved.
>>
>> Signed-off-by: Chencheng Zhang <charles.cc.zhang at gmail.com>
>> ---
>>  net/tftp.c | 33 +++++++++++++++++++++------------
>>  1 file changed, 21 insertions(+), 12 deletions(-)
>>
>> diff --git a/net/tftp.c b/net/tftp.c
>> index 5f2e0a2bc06..5a8094791ab 100644
>> --- a/net/tftp.c
>> +++ b/net/tftp.c
>> @@ -355,20 +355,29 @@ static void tftp_send(void)
>>  		debug("send option \"timeout %s\"\n", (char *)pkt);
>>  		pkt += strlen((char *)pkt) + 1;
>>  #ifdef CONFIG_TFTP_TSIZE
>> -		pkt += sprintf((char *)pkt, "tsize%c%u%c",
>                                                  ^^^
> ... of this format string.

+1, and NACK. The original code is perfectly fine assuming a compliant
snprintf() implementation.

> 
> Are you using USE_TINY_PRINTF or not?
> Do you test with commit 1bc125becaa5 ("tiny-printf: emit \0 as %c")?


Nice hint :)

Thanks,
-- 
Jerome

>> -				0, net_boot_file_size, 0);
>> +		/* Write "tsize" + \0 */
>> +		strcpy((char *)pkt, "tsize");
>> +		pkt += 6; /* 5 chars + 1 null */
>> +
>> +		/* Write the value + \0 */
>> +		pkt += sprintf((char *)pkt, "%u", net_boot_file_size) + 1;
>>  #endif
>> -		/* try for more effic. blk size */
>> -		pkt += sprintf((char *)pkt, "blksize%c%d%c",
>> -				0, tftp_block_size_option, 0);
>>  
>> -		/* try for more effic. window size.
>> -		 * Implemented only for tftp get.
>> -		 * Don't bother sending if it's 1
>> -		 */
>> -		if (tftp_state == STATE_SEND_RRQ && tftp_window_size_option > 1)
>> -			pkt += sprintf((char *)pkt, "windowsize%c%d%c",
>> -					0, tftp_window_size_option, 0);
>> +		/* Write "blksize" + \0 */
>> +		strcpy((char *)pkt, "blksize");
>> +		pkt += 8; /* 7 chars + 1 null */
>> +
>> +		/* Write the value + \0 */
>> +		pkt += sprintf((char *)pkt, "%d", tftp_block_size_option) + 1;
>> +
>> +		if (tftp_state == STATE_SEND_RRQ && tftp_window_size_option > 1) {
>> +			/* Write "windowsize" + \0 */
>> +			strcpy((char *)pkt, "windowsize");
>> +			pkt += 11; /* 10 chars + 1 null */
>> +
>> +			/* Write the value + \0 */
>> +			pkt += sprintf((char *)pkt, "%d", tftp_window_size_option) + 1;
>> +		}
>>  		len = pkt - xp;
>>  		break;
>>  
>