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

On Fri, Oct 14, 2022 at 8:44 PM Rasmus Villemoes rasmus.villemoes@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@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@gmail.com