[PATCH] tftp: make TFTP ports unconditionally configurable

[Quentin Schulz]

Hi Alvin,

On 10/2/25 11:43 AM, Alvin Šipraga wrote:
> [You don't often get email from alvin at pqrs.dk. 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 <alvin at pqrs.dk>
> ---
> ---
>   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.

On another unrelated note, I'm wondering if we shouldn't make the ports 
(tftp_remote_port and tftp_our_port) u16 as that's the max allowed on 
Unix as far as I know.

This would allow to use env_get_ulong() here, e.g.

tftp_remote_port = env_get_ulong("tftpdstp", 10, WELL_KNOWN_PORT);
tftp_our_port = env_get_ulong("tftpsrcp", 10, 1024 + (get_timer(0) % 3072));

But that would only make sense in a separate patch.

Cheers,
Quentin