Hello Alexander,

On 2/20/2025 12:19 AM, Sverdlin, Alexander wrote:
Hi Chintan!

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'.

  #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;

Reply via email to