[PATCH 6/6] net: tftp: sanitize tftp block size, especially for TX

[Ramon Fried]

On Fri, Oct 14, 2022 at 8:44 PM Rasmus Villemoes
<rasmus.villemoes at prevas.dk> wrote:
>
> U-Boot does not support IP fragmentation on TX (and unless
> CONFIG_IP_DEFRAG is set, neither on RX). So the blocks we send must
> fit in a single ethernet packet.
>
> Currently, if tftpblocksize is set to something like 5000 and I
> tftpput a large enough file, U-Boot crashes because we overflow
> net_tx_packet (which only has room for 1500 bytes plus change).
>
> Similarly, if tftpblocksize is set to something larger than what we
> can actually receive (e.g. 50000, with NET_MAXDEFRAG being 16384), any
> tftp get just hangs because we never receive any packets.
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
> ---
>  net/tftp.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>
> diff --git a/net/tftp.c b/net/tftp.c
> index e5e140bcd5..60e1273332 100644
> --- a/net/tftp.c
> +++ b/net/tftp.c
> @@ -708,8 +708,52 @@ static int tftp_init_load_addr(void)
>         return 0;
>  }
>
> +static int saved_tftp_block_size_option;
> +static void sanitize_tftp_block_size_option(enum proto_t protocol)
> +{
> +       int cap, max_defrag;
> +
> +       switch (protocol) {
> +       case TFTPGET:
> +               max_defrag = config_opt_enabled(CONFIG_IP_DEFRAG, CONFIG_NET_MAXDEFRAG, 0);
> +               if (max_defrag) {
> +                       /* Account for IP, UDP and TFTP headers. */
> +                       cap = max_defrag - (20 + 8 + 4);
> +                       /* RFC2348 sets a hard upper limit. */
> +                       cap = min(cap, 65464);
> +                       break;
> +               }
> +               /*
> +                * If not CONFIG_IP_DEFRAG, cap at the same value as
> +                * for tftp put, namely normal MTU minus protocol
> +                * overhead.
> +                */
> +               /* fall through */
> +       case TFTPPUT:
> +       default:
> +               /*
> +                * U-Boot does not support IP fragmentation on TX, so
> +                * this must be small enough that it fits normal MTU
> +                * (and small enough that it fits net_tx_packet which
> +                * has room for PKTSIZE_ALIGN bytes).
> +                */
> +               cap = 1468;
> +       }
> +       if (tftp_block_size_option > cap) {
> +               printf("Capping tftp block size option to %d (was %d)\n",
> +                      cap, tftp_block_size_option);
> +               saved_tftp_block_size_option = tftp_block_size_option;
> +               tftp_block_size_option = cap;
> +       }
> +}
> +
>  void tftp_start(enum proto_t protocol)
>  {
> +       if (saved_tftp_block_size_option) {
> +               tftp_block_size_option = saved_tftp_block_size_option;
> +               saved_tftp_block_size_option = 0;
> +       }
> +
>         if (IS_ENABLED(CONFIG_NET_TFTP_VARS)) {
>                 char *ep;             /* Environment pointer */
>
> @@ -747,6 +791,8 @@ void tftp_start(enum proto_t protocol)
>                 }
>         }
>
> +       sanitize_tftp_block_size_option(protocol);
> +
>         debug("TFTP blocksize = %i, TFTP windowsize = %d timeout = %ld ms\n",
>               tftp_block_size_option, tftp_window_size_option, timeout_ms);
>
> --
> 2.37.2
>
Reviewed-by: Ramon Fried <rfried.dev at gmail.com>