Roll over TFTP block counter to prevent timeouts with data packets

Resolves: rhbz#1869335

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

0268-tftp-increase-blocksize-from-1024-to-1428-octets.patch

@@ -1,50 +0,0 @@
-From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
-From: Javier Martinez Canillas <javierm@redhat.com>
-Date: Thu, 20 Aug 2020 13:19:00 +0200
-Subject: [PATCH] tftp: increase blocksize from 1024 to 1428 octets
-
-The RFC2348 [0] defines how the TFTP Blocksize Option should be negotiated
-between the client and the server. GRUB just hardcodes this to 1024, which
-limits the maximum file size to be downloaded to 67107840 byes, since the
-block number is stored in a 16-bit field.
-
-This limit is a problem when attempting to fetch large files (i.e: initrd
-images) because the block number will overflow, and GRUB will never ack a
-packet. This causes a timeout when trying to download the file over TFTP.
-
-Since the TFTP support in GRUB is used primarily to fetch the kernel and
-initrd images, let's bump the blocksize to 1428 which would allow to get
-larger files.
-
-The value of 1428 is chosen to increase the maximum file size limit, while
-avoiding IP packet fragmentation which could be a problem in some networks.
-
-The TFTP support should be improved to define the blocksize dynamically
-instead of hardcoding it, but until that happens is better to use a value
-that at least allows to fetch kernel and initrd images with the sizes that
-are present in the install media that is used for network booting.
-
-[0]: https://tools.ietf.org/html/rfc2348
-
-Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
----
- grub-core/net/tftp.c | 6 +++---
- 1 file changed, 3 insertions(+), 3 deletions(-)
-
-diff --git a/grub-core/net/tftp.c b/grub-core/net/tftp.c
-index 22badd74316..43920885411 100644
---- a/grub-core/net/tftp.c
-+++ b/grub-core/net/tftp.c
-@@ -322,9 +322,9 @@ tftp_open (struct grub_file *file, const char *filename)
-   rrqlen += grub_strlen ("blksize") + 1;
-   rrq += grub_strlen ("blksize") + 1;
- 
--  grub_strcpy (rrq, "1024");
--  rrqlen += grub_strlen ("1024") + 1;
--  rrq += grub_strlen ("1024") + 1;
-+  grub_strcpy (rrq, "1428");
-+  rrqlen += grub_strlen ("1428") + 1;
-+  rrq += grub_strlen ("1428") + 1;
- 
-   grub_strcpy (rrq, "tsize");
-   rrqlen += grub_strlen ("tsize") + 1;

0268-tftp-roll-over-block-counter-to-prevent-timeouts-wit.patch

@@ -0,0 +1,51 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Javier Martinez Canillas <javierm@redhat.com>
+Date: Mon, 24 Aug 2020 14:46:27 +0200
+Subject: [PATCH] tftp: roll over block counter to prevent timeouts with data
+ packets
+
+The block number is a 16-bit counter which only allows to fetch
+files no bigger than 65535 * blksize. To avoid this limit, the
+counter is rolled over. This behavior isn't defined in RFC 1350
+but is handled by many TFTP servers and it's what GRUB was doing
+before implicitly due an overflow.
+
+Fixing that bug led to TFTP timeouts, since GRUB wasn't acking
+data packets anymore for files with size bigger than the maximum
+mentioned above. Restore the old behavior to prevent this issue.
+
+Resolves: rhbz#1869335
+
+Suggested-by: Peter Jones <pjones@redhat.com>
+Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
+---
+ grub-core/net/tftp.c | 16 ++++++++++++++--
+ 1 file changed, 14 insertions(+), 2 deletions(-)
+
+diff --git a/grub-core/net/tftp.c b/grub-core/net/tftp.c
+index 22badd74316..acbb01c10e7 100644
+--- a/grub-core/net/tftp.c
++++ b/grub-core/net/tftp.c
+@@ -183,8 +183,20 @@ tftp_receive (grub_net_udp_socket_t sock __attribute__ ((unused)),
+ 	  return GRUB_ERR_NONE;
+ 	}
+ 
+-      /* Ack old/retransmitted block. */
+-      if (grub_be_to_cpu16 (tftph->u.data.block) < data->block + 1)
++      /*
++       * Ack old/retransmitted block.
++       *
++       * The block number is a 16-bit counter which only allows to fetch
++       * files no bigger than 65535 * blksize. To avoid this limit, the
++       * counter is rolled over. This behavior isn't defined in RFC 1350
++       * but is handled by many TFTP servers and it's what GRUB was doing
++       * before implicitly due an overflow.
++       *
++       * Fixing that bug led to TFTP timeouts, since GRUB wasn't acking
++       * data packets anymore for files with size bigger than the maximum
++       * mentioned above. Restore the old behavior to prevent this issue.
++       */
++      if (grub_be_to_cpu16 (tftph->u.data.block) < ((data->block + 1) & 0xffffu))
+ 	ack (data, grub_be_to_cpu16 (tftph->u.data.block));
+       /* Ignore unexpected block. */
+       else if (grub_be_to_cpu16 (tftph->u.data.block) > data->block + 1)

grub.patches

@@ -265,4 +265,4 @@ Patch0264: 0264-Fix-const-char-pointers-in-grub-core-net-efi-pxe.c.patch
 Patch0265: 0265-Fix-const-char-pointers-in-grub-core-net-url.c.patch
 Patch0266: 0266-Add-systemd-integration-scripts-to-make-systemctl-re.patch
 Patch0267: 0267-systemd-integration.sh-Also-set-old-menu_show_once-g.patch
-Patch0268: 0268-tftp-increase-blocksize-from-1024-to-1428-octets.patch
+Patch0268: 0268-tftp-roll-over-block-counter-to-prevent-timeouts-wit.patch

grub2.spec

@@ -14,7 +14,7 @@
 Name:		grub2
 Epoch:		1
 Version:	2.04
-Release:	30%{?dist}
+Release:	31%{?dist}
 Summary:	Bootloader with support for Linux, Multiboot and more
 License:	GPLv3+
 URL:		http://www.gnu.org/software/grub/
@@ -516,6 +516,10 @@ rm -r /boot/grub2.tmp/ || :
 %endif
 
 %changelog
+* Mon Aug 31 2020 Javier Martinez Canillas <javierm@redhat.com> - 2.04-31
+- Roll over TFTP block counter to prevent timeouts with data packets
+  Resolves: rhbz#1869335
+
 * Fri Aug 21 2020 Javier Martinez Canillas <javierm@redhat.com> - 2.04-30
 - Set TFTP blocksize to 1428 instead of 2048 to avoid IP fragmentation
   Resolves: rhbz#1869335