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