Commit Graph

10163 Commits

Author SHA1 Message Date
Glenn Washburn
a06549d398 luks2: Convert 8 spaces to tabs
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:04 +01:00
Glenn Washburn
8e8b2316ac misc: Add parentheses around ALIGN_UP() and ALIGN_DOWN() arguments
This ensures that expected order of operations is preserved when arguments
are expressions.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:04 +01:00
Glenn Washburn
880dfd8f40 disk: Rename grub_disk_get_size() to grub_disk_native_sectors()
The function grub_disk_get_size() is confusingly named because it actually
returns a sector count where the sectors are sized in the GRUB native sector
size. Rename to something more appropriate.

Suggested-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:03 +01:00
Glenn Washburn
85307c34e8 loopback: Do not automaticaly replace existing loopback dev, error instead
If there is a loopback device with the same name as the one to be created,
instead of closing the old one and replacing it with the new one, return an
error instead. If the loopback device was created, its probably being used
by something and just replacing it may cause GRUB to crash unexpectedly.
This fixes obvious problems like "loopback d (d)/somefile". Its not too
onerous to force the user to delete the loopback first with the "-d" switch.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:03 +01:00
Glenn Washburn
407ddead69 disk: Move hardcoded max disk size literal to a GRUB_DISK_MAX_SECTORS in disk.h
There is a hardcoded maximum disk size that can be read or written from,
currently set at 1 EiB in grub_disk_adjust_range(). Move the literal into a
macro in disk.h, so our assumptions are more visible. This hard coded limit
does not prevent using larger disks, just GRUB won't read/write past the
limit. The comment accompanying this restriction didn't quite make sense to
me, so its been modified too.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:03 +01:00
Glenn Washburn
8919eecd84 fs: Fix block lists not being able to address to end of disk sometimes
When checking if a block list goes past the end of the disk, make sure
the total size of the disk is in GRUB native sector sizes, otherwise there
will be blocks at the end of the disk inaccessible by block lists.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:03 +01:00
Vladimir Serbinenko
505d92f5e7 mbr: Document new limitations on MBR gap support
Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:03 +01:00
Vladimir Serbinenko
5fd18f77ee mbr: Warn if MBR gap is small and user uses advanced modules
We don't want to support small MBR gap in pair with anything but the
simplest config of biosdisk + part_msdos + simple filesystem. In this
path "simple filesystems" are all current filesystems except ZFS and
Btrfs.

Signed-off-by: Vladimir Serbinenko <phcoder@google.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:03 +01:00
Tianjia Zhang
ba4b3a7b1e efi/tpm: Extract duplicate code into independent functions
Part of the code logic for processing the return value of efi
log_extend_event is repetitive and complicated. Extract the
repetitive code into an independent function.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:03 +01:00
Tianjia Zhang
3ccbaf36d4 efi/tpm: Add debug information for device protocol and eventlog
Add a number of debug logs to the tpm module. The condition tag
for opening debugging is "tpm". On TPM machines, this will bring
great convenience to diagnosis and debugging.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:03 +01:00
Daniel Kiper
a195dbb677 loader/linux: Report the UEFI Secure Boot status to the Linux kernel
Now that the GRUB has a grub_efi_get_secureboot() function to check the
UEFI Secure Boot status, use it to report that to the Linux kernel.

Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Marco A Benatto <mbenatto@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:19:03 +01:00
Javier Martinez Canillas
132ddc42c7 efi: Only register shim_lock verifier if shim_lock protocol is found and SB enabled
The shim_lock module registers a verifier to call shim's verify, but the
handler is registered even when the shim_lock protocol was not installed.

This doesn't cause a NULL pointer dereference in shim_lock_write() because
the shim_lock_init() function just returns GRUB_ERR_NONE if sl isn't set.

But in that case there's no point to even register the shim_lock verifier
since won't do anything. Additionally, it is only useful when Secure Boot
is enabled.

Finally, don't assume that the shim_lock protocol will always be present
when the shim_lock_write() function is called, and check for it on every
call to this function.

Reported-by: Michael Chang <mchang@suse.com>
Reported-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-12 01:17:25 +01:00
Daniel Kiper
d7e54b2e5f efi: Add secure boot detection
Introduce grub_efi_get_secureboot() function which returns whether
UEFI Secure Boot is enabled or not on UEFI systems.

Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Marco A Benatto <mbenatto@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-11 13:56:22 +01:00
Daniel Kiper
ac5c936754 efi: Add a function to read EFI variables with attributes
It will be used to properly detect and report UEFI Secure Boot status to
the x86 Linux kernel. The functionality will be added by subsequent patches.

Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Marco A Benatto <mbenatto@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-11 13:55:31 +01:00
Daniel Kiper
04ae030d0e efi: Return grub_efi_status_t from grub_efi_get_variable()
This is needed to properly detect and report UEFI Secure Boot status
to the x86 Linux kernel. The functionality will be added by subsequent
patches.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Marco A Benatto <mbenatto@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-11 13:54:54 +01:00
Daniel Kiper
f76a27996c efi: Make shim_lock GUID and protocol type public
The GUID will be used to properly detect and report UEFI Secure Boot
status to the x86 Linux kernel. The functionality will be added by
subsequent patches. The shim_lock protocol type is made public for
completeness.

Additionally, fix formatting of four preceding GUIDs.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Signed-off-by: Marco A Benatto <mbenatto@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-11 13:54:23 +01:00
Javier Martinez Canillas
860be435cb arm/term: Fix linking error due multiple ps2_state definitions
When building with --target=arm-linux-gnu --with-platform=coreboot
a linking error occurs caused by multiple definitions of the
ps2_state variable.

Mark them as static since they aren't used outside their compilation unit.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-11 13:53:54 +01:00
Javier Martinez Canillas
97c20f2412 include/grub/i386/linux.h: Include missing <grub/types.h> header
This header uses types defined in <grub/types.h> but does not include it,
which leads to compile errors like the following:

In file included from ../include/grub/cpu/linux.h:19,
                 from kern/efi/sb.c:21:
../include/grub/i386/linux.h:80:3: error: unknown type name ‘grub_uint64_t’
   80 |   grub_uint64_t addr;

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-11 13:53:18 +01:00
Javier Martinez Canillas
b87781feaf i386: Don't include <grub/cpu/linux.h> in coreboot and ieee1275 startup.S
Nothing defined in the header file is used in the assembly code but it
may lead to build errors if some headers are included through this and
contains definitions that are not recognized by the assembler, e.g.:

../include/grub/types.h: Assembler messages:
../include/grub/types.h:76: Error: no such instruction: `typedef signed char grub_int8_t'
../include/grub/types.h:77: Error: no such instruction: `typedef short grub_int16_t'
../include/grub/types.h:78: Error: no such instruction: `typedef int grub_int32_t'

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-12-11 13:52:18 +01:00
Glenn Washburn
6213184b26 luks2: Rename index variable "j" to "i" in luks2_get_keyslot()
Looping variable "j" was named such because the variable name "i" was taken.
Since "i" has been renamed in the previous patch, we can rename "j" to "i".

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 15:33:41 +01:00
Glenn Washburn
f36193ddf1 luks2: Rename variable "i" to "keyslot_idx" in luks2_get_keyslot()
Variables named "i" are usually looping variables. So, rename it to
"keyslot_idx" to ease luks2_get_keyslot() reading.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 15:33:41 +01:00
Glenn Washburn
c28907e235 luks2: Use correct index variable when looping in luks2_get_keyslot()
The loop variable "j" should be used to index the digests and segments json
array, instead of the variable "i", which is the keyslot index.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 15:33:41 +01:00
Glenn Washburn
0eb44d3196 luks2: Rename source disk variable named "disk" to "source" as in luks.c
This makes it more obvious to the reader that the disk referred to is the
source disk, as opposed to say the disk holding the cryptodisk.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 15:33:41 +01:00
Glenn Washburn
d78ce33e60 cryptodisk: Rename "offset" in grub_cryptodisk_t to "offset_sectors"
This makes it clear that the offset represents sectors, not bytes, in
order to improve readability.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 15:33:41 +01:00
Glenn Washburn
535998c2e0 cryptodisk: Rename "total_length" field in grub_cryptodisk_t to "total_sectors"
This creates an alignment with grub_disk_t naming of the same field and is
more intuitive as to how it should be used.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 15:33:41 +01:00
Glenn Washburn
f0715577c2 types: Define GRUB_CHAR_BIT based on compiler macro instead of using literal
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 15:33:40 +01:00
Javier Martinez Canillas
71dd2ef76f include/grub/arm64/linux.h: Include missing <grub/types.h> header
This header uses types defined in <grub/types.h> but does not include it,
which leads to compile errors like the following:

../include/grub/cpu/linux.h:27:3: error: unknown type name ‘grub_uint32_t’
   27 |   grub_uint32_t code0;  /* Executable code */
      |   ^~~~~~~~~~~~~

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 15:33:40 +01:00
Javier Martinez Canillas
11fa824dde include/grub/arm/system.h: Include missing <grub/symbol.h> header
The header uses the EXPORT_FUNC() macro defined in <grub/types.h> but
doesn't include it, which leads to the following compile error on arm:

../include/grub/cpu/system.h:12:13: error: ‘EXPORT_FUNC’ declared as function returning a function
   12 | extern void EXPORT_FUNC(grub_arm_disable_caches_mmu) (void);
      |             ^~~~~~~~~~~
../include/grub/cpu/system.h:12:1: warning: parameter names (without types) in function declaration
   12 | extern void EXPORT_FUNC(grub_arm_disable_caches_mmu) (void);
      | ^~~~~~
make[3]: *** [Makefile:36581: kern/efi/kernel_exec-sb.o] Error 1

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 15:33:40 +01:00
Daniel Axtens
cc316ea337 docs: grub-install --pubkey has been supported for some time
grub-install --pubkey is supported, so we can now document it.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 14:37:21 +01:00
Daniel Axtens
c0c9da5c64 docs: grub-install is no longer a shell script
Since commit cd46aa6cef in 2013, grub-install hasn't been a shell
script. The para doesn't really add that much, especially since it's
the user manual, so just drop it.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-11-20 14:36:50 +01:00
Jacob Kroon
d7cac52eb2 Makefile: Remove unused GRUB_PKGLIBDIR definition
Signed-off-by: Jacob Kroon <jacob.kroon@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-10-30 21:53:00 +01:00
Daniel Axtens
59c8e9fb53 lzma: Fix compilation error under clang 10
Compiling under clang 10 gives:

grub-core/lib/LzmaEnc.c:1362:9: error: misleading indentation; statement is not part of the previous 'if' [-Werror,-Wmisleading-indentation]
        {
        ^
grub-core/lib/LzmaEnc.c:1358:7: note: previous statement is here
      if (repIndex == 0)
      ^
1 error generated.

It's not really that unclear in context: there's a commented-out
if-statement. But tweak the alignment anyway so that clang is happy.

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-10-30 21:53:00 +01:00
Cao jin
2b106a2a4e kern/i386/realmode: Update comment
Commit b81d609e4c did not update it.

Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-10-30 21:53:00 +01:00
Glenn Washburn
6355ba91e5 cryptodisk: Fix cipher IV mode "plain64" always being set as "plain"
When setting cipher IV mode, detection is done by prefix matching the
cipher IV mode part of the cipher mode string. Since "plain" matches
"plain64", we must check for "plain64" first. Otherwise, "plain64" will
be detected as "plain".

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-10-30 15:37:20 +01:00
Glenn Washburn
2df2912266 crypto: Remove GPG_ERROR_CFLAGS from gpg_err_code_t enum
This was probably added by accident when originally creating the file.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-18 22:31:30 +02:00
Glenn Washburn
ac301e4dd0 script: Do not allow a delimiter between function name and block start
Currently the following is valid syntax but should be a syntax error:

  grub> function f; { echo HERE; }
  grub> f
  HERE

This fix is not backward compatible, but current syntax is not documented
either and has no functional value. So any scripts with this unintended
syntax are technically syntactically incorrect and should not be relying
on this behavior.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-18 22:31:30 +02:00
Glenn Washburn
c30f378e00 docs: Support for loading and concatenating multiple initrds
This has been available since January of 2012 but has not been documented.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-18 22:31:30 +02:00
Glenn Washburn
c98a78ae81 lexer: char const * should be const char *
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-18 22:31:30 +02:00
Glenn Washburn
84ff10b1c0 cryptodisk: Use cipher name instead of object in error message
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-18 22:31:30 +02:00
Glenn Washburn
8471d8e254 tests: F2FS test should use MOUNTDEVICE like other tests
LODEVICES is not an array variable and should not be accessed as such.
This allows the f2fs test to pass as it was failing because a device
name had a space prepended to the path.

Signed-off-by: Glenn Washburn <development@efficientek.com>
Acked-by: Jaegeuk Kim <jaegeuk@kernel.org>
Tested-by: Paul Menzel <pmenzel@molgen.mpg.de>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-18 22:31:30 +02:00
Florian La Roche
3ec59f14f4 grub-mkconfig: If $hints is not set reduce the output into grub.cfg to just 1 line
Signed-off-by: Florian La Roche <Florian.LaRoche@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-18 22:31:30 +02:00
Petr Vorel
c55480daca travis: Run bootstrap to fix build
autogen.sh isn't enough:

  $ ./autogen.sh
  Gnulib not yet bootstrapped; run ./bootstrap instead.
  The command "./autogen.sh" exited with 1.

Additionally, using bootstrap requires to install autopoint package.

Signed-off-by: Petr Vorel <pvorel@suse.cz>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-18 22:31:30 +02:00
Patrick Steinhardt
ee12785f75 luks2: Strip dashes off of the UUID
The UUID header for LUKS2 uses a format with dashes, same as for
LUKS(1). But while we strip these dashes for the latter, we don't for
the former. This isn't wrong per se, but it's definitely inconsistent
for users as they need to use the dashed format for LUKS2 and the
non-dashed format for LUKS when e.g. calling "cryptomount -u $UUID".

Fix this inconsistency by stripping dashes off of the LUKS2 UUID.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-18 22:31:29 +02:00
Tianjia Zhang
6efd04f314 efi/tpm: Remove unused functions and structures
Although the tpm_execute() series of functions are defined they are not
used anywhere. Several structures in the include/grub/efi/tpm.h header
file are not used too. There is even nonexistent grub_tpm_init()
declaration in this header. Delete all that unneeded stuff.

If somebody needs the functionality implemented in the dropped code then
he/she can re-add it later. Now it needlessly increases the GRUB
code/image size.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-18 22:31:29 +02:00
Tianjia Zhang
de094060ac shim_lock: Enable module for all EFI architectures
Like the tpm the shim_lock module is only enabled for x86_64 target.
However, there's nothing specific to x86_64 in the implementation and
it can be enabled for all EFI architectures.

Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-18 22:26:48 +02:00
Daniel Kiper
1e81bf6d2d efi/tpm: Fix typo in grub_efi_tpm2_protocol struct
Rename get_active_pcr_blanks() to get_active_pcr_banks().

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
2020-09-18 21:23:21 +02:00
Daniel Kiper
c386331364 i386/efi/init: Drop bogus include
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
2020-09-18 21:22:32 +02:00
Daniel Kiper
30c4e3ca40 docs: Fix devicetree command description
Specifically fix the subsection and drop bogus reference to the GNU/Linux.

Reported-by: Patrick Higgins <higgi1pt@gmail.com>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
2020-09-18 21:20:43 +02:00
Martin Whitaker
8451454889 grub-install: Fix inverted test for NLS enabled when copying locales
Commit 3d8439da8 (grub-install: Locale depends on nls) attempted to avoid
copying locale files to the target directory when NLS was disabled.
However the test is inverted, and it does the opposite.

Signed-off-by: Martin Whitaker <fsf@martin-whitaker.me.uk>
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
2020-09-18 21:17:07 +02:00
Javier Martinez Canillas
a6838bbc67 tftp: Roll-over block counter to prevent data packets timeouts
Commit 781b3e5efc (tftp: Do not use priority queue) caused a regression
when fetching files over TFTP whose size is bigger than 65535 * block size.

  grub> linux /images/pxeboot/vmlinuz
  grub> echo $?
  0
  grub> initrd /images/pxeboot/initrd.img
  error: timeout reading '/images/pxeboot/initrd.img'.
  grub> echo $?
  28

It is caused by the block number counter being a 16-bit field, which leads
to a maximum file size of ((1 << 16) - 1) * block size. Because GRUB sets
the block size to 1024 octets (by using the TFTP Blocksize Option from RFC
2348 [0]), the maximum file size that can be transferred is 67107840 bytes.

The TFTP PROTOCOL (REVISION 2) RFC 1350 [1] does not mention what a client
should do when a file size is bigger than the maximum, but most TFTP hosts
support the block number counter to be rolled over. That is, acking a data
packet with a block number of 0 is taken as if the 65356th block was acked.

It was working before because the block counter roll-over was happening due
an overflow. But that got fixed by the mentioned commit, which led to the
regression when attempting to fetch files larger than the maximum size.

To allow TFTP file transfers of unlimited size again, re-introduce a block
counter roll-over so the data packets are acked preventing the timeouts.

[0]: https://tools.ietf.org/html/rfc2348
[1]: https://tools.ietf.org/html/rfc1350

Fixes: 781b3e5efc (tftp: Do not use priority queue)

Suggested-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
2020-09-11 15:52:07 +02:00