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

[Chencheng Zhang]

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.
BR/Chencheng

On Mon, 23 Mar 2026 at 13:10, Jerome Forissier <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>
> >> ---
> >>  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;
> >>
> >
>
>