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

[Jerome Forissier]

On 23/03/2026 14:51, Chencheng Zhang wrote:
> Hi Sverdin and Jerome,
> 
>> Do you test with commit 1bc125becaa5 ("tiny-printf: emit \0 as %c")?
> Thanks for pointing that out!
> I have checked my baseline which is TI u-boot v2025-01. Clearly 1bc125becaa5 is not included. I patched my baseline with 1bc125becaa5 and it works.
> 
> You can reject my patch.

OK. Thanks for contributing nevertheless.

-- 
Jerome

> BR/Chencheng
> 
> On Mon, 23 Mar 2026 at 13:10, Jerome Forissier <jerome.forissier at arm.com <mailto:jerome.forissier at arm.com>> wrote:
> 
> 
> 
>     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 <mailto: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;
>     >> 
>     >
>