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'.
Does it produce \000 octet between tsize and net_boot_file_size value?
I suppose TFTP protocol requires it?
Looking into _vprintf() implementation, I'd say it would have problems
emitting %c with parameter 0, no matter if at the end of the string, or
in the middle.
Would the following patch fix the problem you observe?
diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
index 0503c17341f..0e620b272d8 100644
--- a/lib/tiny-printf.c
+++ b/lib/tiny-printf.c
@@ -299,7 +299,7 @@ static int _vprintf(struct printf_info *info, const char
*fmt, va_list va)
}
break;
case 'c':
- out(info, (char)(va_arg(va, int)));
+ info->putc(info, (char)(va_arg(va, int)));
break;
case 's':
p = va_arg(va, char*);
--
Alexander Sverdlin
Siemens AG
www.siemens.com
