Hi Chintan! On Thu, 2025-02-20 at 01:29 +0530, Vankar, Chintan wrote: > > On Wed, 2025-02-19 at 16:18 +0530, Chintan Vankar wrote: > > > To append a string to a tftp pkt, "tftp_send()" API invokes "sprintf()" > > > function which copies a string excluding a null character causing TFTP > > > not-null terminated string error. Increase TFTP pkt string by 1 to avoid > > > > Is this error visible somehow? How did you stop this problem? > > > > Yes this error is visible while loading the file via TFTP. I tried > tracing back the error and it was due to string copying in "tftp.c" file > that I modified. > > Since commit 'e2d96ac9ee9f81c8f72435bff55f924d27ad92d1' has modified > sprintf() function to return the string length instead of 0 so it was > not seen before. Also from: > https://github.com/u-boot/u-boot/blob/master/net/tftp.c#L347, > you can see that we are incrementing pkt length by 1 on every sprintf() > call but that was not done for the later part that I modified and that's > how I come to this approach. > > > > this error. > > > > > > Signed-off-by: Chintan Vankar <c-van...@ti.com> > > > --- > > > > > > Link to v1: > > > https://lore.kernel.org/r/20250107093840.2211381-2-c-van...@ti.com/ > > > > > > Changes from v1 to v2: > > > -> Updated commit message. > > > > > > net/tftp.c | 6 +++--- > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > diff --git a/net/tftp.c b/net/tftp.c > > > index fd9c9492929..420ea9ecf6c 100644 > > > --- a/net/tftp.c > > > +++ b/net/tftp.c > > > @@ -347,11 +347,11 @@ static void tftp_send(void) > > > pkt += strlen((char *)pkt) + 1; > > > #ifdef CONFIG_TFTP_TSIZE > > > pkt += sprintf((char *)pkt, "tsize%c%u%c", > > > - 0, net_boot_file_size, 0); > > > + 0, net_boot_file_size, 0) + 1; > > > > But it does indeed produce \000 octets, because of %c with "0" argument, > > doesn't it? > > > > While debugging I tried to debug the pkt string and I come to know that > before this point 'null character' was getting appended because of > incrementing a pkt length by one, but not after this. It was just > appending a string, so it was not producing a '\0'.
I've made some test on my side and can confirm that: - the above fix is not correct because the output is actually short by 2, not by one in the above case (two times %c missing) - the root cause lies in tiny-printf, not in TFTP code I believe this is one of the ways to fix the root cause: https://lists.denx.de/pipermail/u-boot/2025-February/581142.html > > > #endif > > > /* try for more effic. blk size */ > > > pkt += sprintf((char *)pkt, "blksize%c%d%c", > > > - 0, tftp_block_size_option, 0); > > > + 0, tftp_block_size_option, 0) + 1; > > > > > > /* try for more effic. window size. > > > * Implemented only for tftp get. > > > @@ -359,7 +359,7 @@ static void tftp_send(void) > > > */ > > > 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); > > > + 0, tftp_window_size_option, 0) + 1; > > > len = pkt - xp; > > > break; -- Alexander Sverdlin Siemens AG www.siemens.com
