On 10/6/25 4:22 PM, Quentin Schulz wrote:
Hi Alvin,

On 10/2/25 11:43 AM, Alvin Šipraga wrote:
[You don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

A few lines of code being guarded by the CONFIG_TFTP_PORT option seems
an unnecessary restriction on the TFTP support provided by a vanilla
U-Boot image. In cases where the TFTP server cannot run as superuser -
and hence cannot run on the well-known port 69 - this quirk incurs a
full reconfiguration and rebuild of the bootloader only in order to
select the appropriate destination port.

Remove the CONFIG_TFTP_PORT option entirely and make the tftpdstp and
tftpsrcp variables always have an effect. Their being unset will mean
that U-Boot behaves the same as if CONFIG_TFTP_PORT was unset. Update
the documentation accordingly. And fix up the single board which was
originally enabling this option.

Signed-off-by: Alvin Šipraga <[email protected]>
---
---
  configs/gurnard_defconfig |  1 -
  doc/usage/cmd/tftpput.rst |  8 ++------
  net/Kconfig               | 18 ------------------
  net/tftp.c                |  3 +--
  4 files changed, 3 insertions(+), 27 deletions(-)

diff --git a/configs/gurnard_defconfig b/configs/gurnard_defconfig
index bde531a6003a2d7167767b3f509876137c7da1a8..85643f249aed65071005a157658c3dc11f44c619 100644
--- a/configs/gurnard_defconfig
+++ b/configs/gurnard_defconfig
@@ -42,7 +42,6 @@ CONFIG_ENV_OVERWRITE=y
  CONFIG_ENV_IS_IN_NAND=y
  CONFIG_ENV_RELOC_GD_ENV_ADDR=y
  CONFIG_NET_RETRY_COUNT=20
-CONFIG_TFTP_PORT=y
  CONFIG_TFTP_TSIZE=y
  CONFIG_AT91_GPIO=y
  CONFIG_GENERIC_ATMEL_MCI=y
diff --git a/doc/usage/cmd/tftpput.rst b/doc/usage/cmd/tftpput.rst
index 351c9faa38b9d8357c0e200d8cebd1f90344e0a6..2bcb3032cb2444926da28454eefdb2b128a7389f 100644
--- a/doc/usage/cmd/tftpput.rst
+++ b/doc/usage/cmd/tftpput.rst
@@ -19,9 +19,8 @@ Description
  The tftpput command is used to transfer a file to a TFTP server.

  By default the destination port is 69 and the source port is pseudo- random. -If CONFIG_TFTP_PORT=y, the environment variable *tftpsrcp* can be used to set -the source port and the environment variable *tftpdstp* can be used to set
-the destination port.
+The environment variable *tftpsrcp* can be used to set the source port and the
+environment variable *tftpdstp* can be used to set the destination port.

  address
      memory address where the data starts
@@ -75,9 +74,6 @@ The command is only available if CONFIG_CMD_TFTPPUT=y.
  CONFIG_TFTP_BLOCKSIZE defines the size of the TFTP blocks sent. It defaults
  to 1468 matching an ethernet MTU of 1500.

-If CONFIG_TFTP_PORT=y, the environment variables *tftpsrcp* and *tftpdstp* can
-be used to set the source and the destination ports.
-
  CONFIG_TFTP_WINDOWSIZE can be used to set the TFTP window size of transmits
  after which an ACK response is required. The window size defaults to 1.

diff --git a/net/Kconfig b/net/Kconfig
index 40ec6bbce7614afde5a2a9f90d431f19a3654a41..7ba64d43b398415825921e7b61dc4c81e986618a 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -60,24 +60,6 @@ config SYS_FAULT_ECHO_LINK_DOWN
           this option is active, then CONFIG_SYS_FAULT_MII_ADDR also needs to
           be configured.

-config TFTP_PORT
-       bool "Set TFTP UDP source/destination ports via the environment"
-       help
-         If this is defined, the environment variable tftpsrcp is used to -         supply the TFTP UDP source port value.  If tftpsrcp isn't defined,
-         the normal pseudo-random port number generator is used.
-
-         Also, the environment variable tftpdstp is used to supply the TFTP -         UDP destination port value.  If tftpdstp isn't defined, the normal
-         port 69 is used.
-
-         The purpose for tftpsrcp is to allow a TFTP server to blindly start -         the TFTP transfer using the pre-configured target IP address and UDP -         port. This has the effect of "punching through" the (Windows XP) -         firewall, allowing the remainder of the TFTP transfer to proceed -         normally.  A better solution is to properly configure the firewall,
-         but sometimes that is not allowed.
-
  config TFTP_WINDOWSIZE
         int "TFTP window size"
         default 1
diff --git a/net/tftp.c b/net/tftp.c
index 1ca9a5ea7cf6bac562006e2506ebcb17996a8c36..1760877107faef57107cb04c484157750ff63602 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -926,14 +926,13 @@ void tftp_start(enum proto_t protocol)
         /* Use a pseudo-random port unless a specific port is set */
         tftp_our_port = 1024 + (get_timer(0) % 3072);

-#ifdef CONFIG_TFTP_PORT
         ep = env_get("tftpdstp");
         if (ep != NULL)
                 tftp_remote_port = simple_strtol(ep, NULL, 10);
         ep = env_get("tftpsrcp");
         if (ep != NULL)
                 tftp_our_port = simple_strtol(ep, NULL, 10);
-#endif
+

The only worry I have with this patch is the security implications of that. Indeed, if I manage to get access to the host (root not required) where the TFTP server is running on port 69 AND the environment of the board, I could set tftpdstp to point to the TFTP server I (an attacker) just started on a non-privileged port.


Thinking about this five more seconds, I guess at that point, one could replace the command calling tftp directly and point U-Boot to a different IP for the TFTP server or do other nastier things.

If you're blindly trusting the environment from an external source, it's your problem anyway, simply allow importing some variables to be imported from other environments and limit the exposure that way.

So I guess it's fine as is for this patch,

Reviewed-by: Quentin Schulz <[email protected]>

Thanks!
Quentin

Reply via email to