2021-06-08 Daniel Kiper Release 2.06 2021-06-08 Daniel Kiper SECURITY: Add SECURITY file The SECURITY file describes the GRUB project security policy. It is based on https://github.com/wireapp/wire/blob/master/SECURITY.md 2021-06-08 Daniel Kiper MAINTAINERS: Add MAINTAINERS file The MAINTAINERS file provides basic information about the GRUB project and its maintainers. 2021-06-01 Dimitri John Ledkov grub-install: Add backup and restore Refactor clean_grub_dir() to create a backup of all the files, instead of just irrevocably removing them as the first action. If available, register atexit() handler to restore the backup if errors occur before point of no return, or remove the backup if everything was successful. If atexit() is not available, the backup remains on disk for manual recovery. Some platforms defined a point of no return, i.e. after modules & core images were updated. Failures from any commands after that stage are ignored, and backup is cleaned up. For example, on EFI platforms update is not reverted when efibootmgr fails. Extra care is taken to ensure atexit() handler is only invoked by the parent process and not any children forks. Some older GRUB codebases can invoke parent atexit() hooks from forks, which can mess up the backup. This allows safer upgrades of MBR & modules, such that modules/images/fonts/translations are consistent with MBR in case of errors. For example accidental grub-install /dev/non-existent-disk currently clobbers and upgrades modules in /boot/grub, despite not actually updating any MBR. This patch only handles backup and restore of files copied to /boot/grub. This patch does not perform backup (or restoration) of MBR itself or blocklists. Thus when installing i386-pc platform, corruption may still occur with MBR and blocklists which will not be attempted to be automatically recovered. Also add modinfo.sh and *.efi to the cleanup/backup/restore code path, to ensure it is also cleaned, backed up and restored. Reviewed-by: Daniel Kiper 2021-06-01 Dimitri John Ledkov osdep/unix/exec: Avoid atexit() handlers when child execvp() fails The functions grub_util_exec_pipe() and grub_util_exec_pipe_stderr() currently call execvp(). If the call fails for any reason, the child currently calls exit(127). This in turn executes the parents atexit() handlers from the forked child, and then the same handlers are called again from parent. This is usually not desired, and can lead to deadlocks, and undesired behavior. So, change the exit() calls to _exit() calls to avoid calling atexit() handlers from child. Fixes: e75cf4a58 (unix exec: avoid atexit handlers when child exits) Reviewed-by: Daniel Kiper 2021-06-01 Jan (janneke) Nieuwenhuizen lib/i386/relocator64: Build fixes for i386 This fixes cross-compiling to x86 (e.g., the Hurd) from x86-linux of grub-core/lib/i386/relocator64.S This file has six sections that only build with a 64-bit assembler, yet only the first two sections had support for a 32-bit assembler. This patch completes this for the remaining sections. To reproduce, update the GRUB source description in your local Guix archive and run ./pre-inst-env guix build --system=i686-linux --target=i586-pc-gnu grub or install an x86 cross-build environment on x86-linux (32-bit!) and configure to cross build and make, e.g., do something like ./configure \ CC_FOR_BUILD=gcc \ --build=i686-unknown-linux-gnu \ --host=i586-pc-gnu make Additionally, remove a line with redundant spaces. Reviewed-by: Daniel Kiper 2021-06-01 Javier Martinez Canillas fs/xfs: Add needsrepair incompat feature support The XFS now has an incompat feature flag to indicate that a filesystem needs to be repaired. The Linux kernel refuses to mount the filesystem that has it set and only the xfs_repair tool is able to clear that flag. The GRUB doesn't have the concept of mounting filesystems and just attempts to read the files. But it does some sanity checking before attempting to read from the filesystem. Among the things which are tested, is if the super block only has set of incompatible features flags that are supported by GRUB. If it contains any flags that are not listed as supported, reading the XFS filesystem fails. Since the GRUB doesn't attempt to detect if the filesystem is inconsistent nor replays the journal, the filesystem access is a best effort. For this reason, ignore if the filesystem needs to be repaired and just print a debug message. That way, if reading or booting fails later, the user is able to figure out that the failures can be related to broken XFS filesystem. Suggested-by: Eric Sandeen Reviewed-by: Daniel Kiper 2021-06-01 Carlos Maiolino fs/xfs: Add bigtime incompat feature support The XFS filesystem supports a bigtime feature to overcome y2038 problem. This patch makes the GRUB able to support the XFS filesystems with this feature enabled. The XFS counter for the bigtime enabled timestamps starts at 0, which translates to GRUB_INT32_MIN (Dec 31 20:45:52 UTC 1901) in the legacy timestamps. The conversion to Unix timestamps is made before passing the value to other GRUB functions. For this to work properly, GRUB requires an access to flags2 field in the XFS ondisk inode. So, the grub_xfs_inode structure has been updated to cover full ondisk inode. Reviewed-by: Daniel Kiper 2021-06-01 Carlos Maiolino fs: Use 64-bit type for filesystem timestamp Some filesystems nowadays use 64-bit types for timestamps. So, update grub_dirhook_info struct to use an grub_int64_t type to store mtime. This also updates the grub_unixtime2datetime() function to receive a 64-bit timestamp argument and do 64-bit-safe divisions. All the remaining conversion from 32-bit to 64-bit should be safe, as 32-bit to 64-bit attributions will be implicitly casted. The most critical part in the 32-bit to 64-bit conversion is in the function grub_unixtime2datetime() where it needs to deal with the 64-bit type. So, for that, the grub_divmod64() helper has been used. These changes enables the GRUB to support dates beyond y2038. Reviewed-by: Daniel Kiper 2021-05-28 Javier Martinez Canillas types: Define PRI{x,d}GRUB_INT{32,64}_T format specifiers There are already PRI*_T constants defined for unsigned integers but not for signed integers. Add format specifiers for the latter. Suggested-by: Daniel Kiper Reviewed-by: Daniel Kiper 2021-05-28 Tianjia Zhang kern/efi/sb: Remove duplicate efi_shim_lock_guid variable The efi_shim_lock_guid local variable and shim_lock_guid global variable have the same GUID value. Only the latter is retained. Reviewed-by: Daniel Kiper 2021-05-10 Javier Martinez Canillas util/mkimage: Fix wrong PE32+ section sizes for some arches The commit f60ba9e5945 (util/mkimage: Refactor section setup to use a helper) added a helper function to setup PE sections. But it also changed how the raw data offsets were calculated since all the section sizes are aligned. However, for some platforms, i.e ia64-efi and arm64-efi, the kernel image size is not aligned using the section alignment. This leads to the situation in which the mods section offset in its PE section header does not match its real placement in the PE file. So, finally the GRUB is not able to locate and load built-in modules. The problem surfaces on ia64-efi and arm64-efi because both platforms require additional relocation data which is added behind .bss section. So, we have to add some padding behind this extra data to make the beginning of mods section properly aligned in the PE file. Fix it by aligning the kernel_size to the section alignment. That makes the sizes and offsets in the PE section headers to match relevant sections in the PE32+ binary file. Reported-by: John Paul Adrian Glaubitz Tested-by: John Paul Adrian Glaubitz Reviewed-by: Daniel Kiper 2021-05-10 Daniel Kiper term/terminfo: Fix the terminfo command help and documentation Additionally, fix the terminfo spelling mistake in the GRUB development documentation. Reviewed-by: Javier Martinez Canillas 2021-05-10 Daniel Kiper i18n: Align N_() formatting with the rest of GRUB code Reviewed-by: Javier Martinez Canillas 2021-05-10 Daniel Kiper i18n: Format large integers before the translation message - take 2 This is an additional fix which has been missing from the commit 837fe48de (i18n: Format large integers before the translation message). Reviewed-by: Javier Martinez Canillas 2021-04-13 Miguel Ángel Arruga Vivas i18n: Format large integers before the translation message The GNU gettext only supports the ISO C99 macros for integral types. If there is a need to use unsupported formatting macros, e.g. PRIuGRUB_UINT64_T, according to [1] the number to a string conversion should be separated from the code printing message requiring the internationalization. So, the function grub_snprintf() is used to print the numeric values to an intermediate buffer and the internationalized message contains a string format directive. [1] https://www.gnu.org/software/gettext/manual/html_node/Preparing-Strings.html#No-string-concatenation Reviewed-by: Daniel Kiper 2021-04-12 Daniel Axtens video/fb/fbfill: Use unsigned integers for width/height Since commit 7ce3259f67ac (video/fb/fbfill: Fix potential integer overflow), clang builds of grub-emu have failed with messages like: /usr/bin/ld: libgrubmods.a(libgrubmods_a-fbfill.o): in function `grub_video_fbfill_direct24': fbfill.c:(.text+0x28e): undefined reference to `__muloti4' This appears to be due to a weird quirk in how clang compiles grub_mul(dst->mode_info->bytes_per_pixel, width, &rowskip) which is grub_mul(unsigned int, int, &grub_size_t). It looks like clang somewhere promotes everything to 128-bit maths before ultimately reducing down to 64 bit for grub_size_t. I think this is because width is signed, and indeed converting width to an unsigned int makes the problem go away. This conversion also makes more sense generally: - the caller of all the fbfill_directN functions is grub_video_fb_fill_dispatch() and it takes width and height as unsigned ints already, - it doesn't make sense to fill a negative width or height. Convert the width and height arguments and associated loop counters to unsigned ints. Fixes: 7ce3259f67ac (video/fb/fbfill: Fix potential integer overflow) Reviewed-by: Daniel Kiper 2021-04-12 Glenn Washburn docs: Conform badmem and cutmem description indentations with other commands Reviewed-by: Daniel Kiper docs: Add note to cryptomount that UUIDs should be specified without dashes Reviewed-by: Daniel Kiper 2021-04-12 Aru Sahni templates: Fix user-facing typo with an incorrect use of "it's" Since the possessive form of "it" is being used, the apostrophe must be omitted. Reviewed-by: Daniel Kiper 2021-04-12 Colin Watson buffer: Sync up out-of-range error message The messages associated with other similar GRUB_ERR_OUT_OF_RANGE errors were lacking the trailing full stop. Syncing up the strings saves a small amount of precious core image space on i386-pc. DOWN: obj/i386-pc/grub-core/kernel.img (31740 > 31708) - change: -32 DOWN: i386-pc core image (biosdisk ext2 part_msdos) (27453 > 27452) - change: -1 DOWN: i386-pc core image (biosdisk ext2 part_msdos diskfilter mdraid09) (32367 > 32359) - change: -8 Reviewed-by: Daniel Kiper 2021-04-12 Glenn Washburn usb/usbhub: Use GRUB_USB_MAX_CONF macro instead of literal in hub for maximum configs Reviewed-by: Daniel Kiper 2021-04-12 Daniel Drake fs/minix: Avoid mistakenly probing ext2 filesystems The ext2 (and ext3, ext4) filesystems write the number of free inodes to location 0x410. On a MINIX filesystem, that same location is used for the MINIX superblock magic number. If the number of free inodes on an ext2 filesystem is equal to any of the four MINIX superblock magic values plus any multiple of 65536, GRUB's MINIX filesystem code will probe it as a MINIX filesystem. In the case of an OS using ext2 as the root filesystem, since there will ordinarily be some amount of file creation and deletion on every bootup, it effectively means that this situation has a 1:16384 chance of being hit on every reboot. This will cause GRUB's filesystem probing code to mistakenly identify an ext2 filesystem as MINIX. This can be seen by e.g. "search --label" incorrectly indicating that no such ext2 partition with matching label exists, whereas in fact it does. After spotting the rough cause of the issue I was facing here, I borrowed much of the diagnosis/explanation from meierfra who found and investigated the same issue in util-linux in 2010: https://bugs.launchpad.net/ubuntu/+source/util-linux/+bug/518582 This was fixed in util-linux by having the MINIX code check for the ext2 magic. Do the same here. Reviewed-by: Derek Foreman Reviewed-by: Daniel Kiper 2021-03-12 Daniel Kiper Release 2.06~rc1 2021-03-11 Ard Biesheuvel arm/linux: Fix ARM Linux header layout The hdr_offset member of the ARM Linux image header appears at offset 0x3c, matching the PE/COFF spec's placement of the COFF header offset in the MS-DOS header. We're currently off by four, so fix that. Reviewed-by: Daniel Kiper 2021-03-10 Glenn Washburn style: Format string macro should have a space between quotes Reviewed-by: Daniel Kiper 2021-03-10 Glenn Washburn grub/err: Do compile-time format string checking on grub_error() This should help prevent format string errors and thus improve the quality of error reporting. Reviewed-by: Daniel Kiper 2021-03-10 Glenn Washburn fs/zfs/zfs: Use format code "%llu" for 64-bit uint bp->blk_prop in grub_error() This is a temporary, less-intrusive change to get the build to success with compiler format string checking turned on. There is a better fix which addresses this issue, but it needs more testing. Use this change so that format string checking on grub_error() can be turned on until the better change is fully tested. Reviewed-by: Daniel Kiper 2021-03-10 Glenn Washburn fs/hfsplus: Use format code PRIuGRUB_UINT64_T for 64-bit typed fileblock in grub_error() Reviewed-by: Daniel Kiper 2021-03-10 Glenn Washburn dl/elf: Use format code PRIxGRUB_UINT64_T for 64-bit arg in grub_error() The macro ELF_R_TYPE does not change the underlying type. Here its argument is a 64-bit Elf64_Xword. Make sure the format code matches. For the RISC-V architecture, rel->r_info could be either Elf32_Xword or Elf64_Xword depending on if 32 or 64-bit RISC-V is being built. So cast to 64-bit value regardless. Reviewed-by: Daniel Kiper 2021-03-10 Glenn Washburn disk/ata: Use format code PRIxGRUB_UINT64_T for 64-bit uint argument in grub_error() Reviewed-by: Daniel Kiper 2021-03-10 Glenn Washburn loader/i386/pc/linux: Use PRI* macros to get correct format string code across architectures Also remove casting of format string args so that the architecture dependent type is preserved. Reviewed-by: Daniel Kiper 2021-03-10 Glenn Washburn kern/efi/mm: Format string error in grub_error() The second format string argument, GRUB_EFI_MAX_USABLE_ADDRESS, is a macro to a number literal. However, depending on what the target architecture, the type can be 32 or 64 bits. Cast to a 64-bit integer. Also, change the format string literals "%llx" to use PRIxGRUB_UINT64_T. Reviewed-by: Daniel Kiper 2021-03-10 Glenn Washburn commands/pgp: Format code for grub_error() is incorrect The format code is for a 32-bit int, but the argument, keyid, is declared as a 64 bit int. The comment above says keyid is 32-bit. I'm not sure if the comment or declaration is wrong, so force the display of a 64-bit int for now. Reviewed-by: Daniel Kiper 2021-03-10 Glenn Washburn grub_error: Use format code PRIuGRUB_SIZE for variables of type grub_size_t Reviewed-by: Daniel Kiper 2021-03-10 Glenn Washburn disk/dmraid_nvidia: Format string error in grub_error() The grub_error() has a format string expecting two arguments, but only one provided. According to the comments in the struct grub_nv_super definition, the version field looks like a version number where major.minor is encoded as each a byte in the two-byte short. Reviewed-by: Daniel Kiper 2021-03-10 Glenn Washburn video/bochs: grub_error() format string add missing format code Reviewed-by: Daniel Kiper 2021-03-10 Glenn Washburn parttool/msdospart: grub_error() missing format string argument Its obvious from the error message that the variable named "type" was accidentally omitted. Reviewed-by: Daniel Kiper 2021-03-10 Glenn Washburn misc: Format string for grub_error() should be a literal Reviewed-by: Daniel Kiper 2021-03-10 Philip Müller templates: Properly disable the os-prober by default This patch does the following: - really disables os-prober by default in the util/grub-mkconfig.in by setting GRUB_DISABLE_OS_PROBER to true, - fixes the logic in the util/grub.d/30_os-prober.in, - updates the grub_warn() lines. Reason for the code shuffling in the util/grub-mkconfig.in: The default was GRUB_DISABLE_OS_PROBER=false if you don't set GRUB_DISABLE_OS_PROBER at all. To prevent os-prober from starting we have to set it by default to true and shuffle GRUB_DISABLE_OS_PROBER to code section, which is executed by the script. However we still give an option to the user to overwrite it with false, if he wants to execute os-prober after all. Fixes: e3464147 (templates: Disable the os-prober by default) Reported-by: Didier Spaier Reported-by: Lennart Sorensen Reported-by: John Paul Adrian Glaubitz Reviewed-by: Daniel Kiper 2021-03-10 Michael Chang kern/efi/sb: Add chainloaded image as shim's verifiable object While attempting to dual boot Microsoft Windows with UEFI chainloader, it failed with below error when UEFI Secure Boot was enabled: error ../../grub-core/kern/verifiers.c:119:verification requested but nobody cares: /EFI/Microsoft/Boot/bootmgfw.efi. It is a regression, as previously it worked without any problem. It turns out chainloading PE image has been locked down by commit 578c95298 (kern: Add lockdown support). However, we should consider it as verifiable object by shim to allow booting in UEFI Secure Boot mode. The chainloaded PE image could also have trusted signature created by vendor with their pubkey cert in db. For that matters it's usage should not be locked down under UEFI Secure Boot, and instead shim should be allowed to validate a PE binary signature before running it. Fixes: 578c95298 (kern: Add lockdown support) Reviewed-by: Daniel Kiper 2021-03-10 Glenn Washburn disk/pata: Suppress error message "no device connected" This error message comes from the grub_print_error() in grub_pata_device_initialize(), which does not pass on the error, and is raised in check_device(). The function check_device() needs to return this as an error because check_device() is also used in grub_pata_open(), which does pass on this error to indicate that the device can not be used. This is actually not an error when displayed by grub_pata_device_initialize() because it just indicates that there are no pata devices seen. This may be confusing to end users who do not have pata devices yet are loading the pata module (perhaps implicitly via nativedisk). This also causes unnecessary output which may need to be accounted for in functional testing. Instead print to the debug log when check_device() raises this "error" and pop the error from the error stack. If there is another error on the stack then print the error stack as those should be real errors. Acked-by: Paul Menzel Reviewed-by: Daniel Kiper 2021-03-10 Yi Zhao fs/ext2: Fix a file not found error when a symlink filesize is equal to 60 We encountered a file not found error when the symlink filesize is equal to 60: $ ls -l initrd lrwxrwxrwx 1 root root 60 Jan 6 16:37 initrd -> secure-core-image-initramfs-5.10.2-yoctodev-standard.cpio.gz When booting, we got the following error in the GRUB: error: file `/initrd' not found The root cause is that the size of diro->inode.symlink is equal to 60 and a symlink name has to be terminated with NUL there. So, if the symlink filesize is exactly 60 then it is also stored in a separate block rather than in the inode itself. Reviewed-by: Daniel Kiper 2021-03-02 Tianjia Zhang loader/i386/linux: Do not use grub_le_to_cpu32() for relocatable variable The relocatable variable is defined as grub_uint8_t. Relevant member in setup_header structure is also defined as one byte in Linux boot protocol. By semantic definition it is a bool type. It is not appropriate to treat it as a four bytes. This patch fixes the issue. Reviewed-by: Daniel Kiper 2021-03-02 Tianjia Zhang loader/i386/linux: Remove redundant code from in grub_cmd_linux() The preferred_address has been assigned to GRUB_LINUX_BZIMAGE_ADDR during initialization in grub_cmd_linux(). The assignment here is redundant and should be removed. Reviewed-by: Daniel Kiper 2021-03-02 Heinrich Schuchardt efi: The device-tree must be in EfiACPIReclaimMemory According to the Embedded Base Boot Requirements (EBBR) specification the device-tree passed to Linux as a configuration table must reside in EfiACPIReclaimMemory. Reviewed-by: Daniel Kiper 2021-03-02 Heinrich Schuchardt commands/efi/lsefisystab: Add short text for EFI_RT_PROPERTIES_TABLE_GUID UEFI specification 2.8 errata B introduced the EFI_RT_PROPERTIES_TABLE describing the services available at runtime. The lsefisystab command is used to display installed EFI configuration tables. Currently it only shows the GUID but not a short text for the new table. Provide a short text for the EFI_RT_PROPERTIES_TABLE_GUID. Reviewed-by: Daniel Kiper 2021-03-02 Petr Vorel docs/luks2: Mention key derivation function support To give users hint why Argon2, the default in cryptsetup for LUKS2, does not work. Acked-by: Paul Menzel Reviewed-by: Patrick Steinhardt Reviewed-by: Daniel Kiper 2021-03-02 Derek Foreman commands/file: Fix array/enum desync The commit f1957dc8a (RISC-V: Add to build system) added two entries to the options array, but only 1 entry to the enum. This resulted in everything after the insertion point being off by one. This broke at least the "file --is-hibernated-hiberfil" command. Bring the two back in sync by splitting the IS_RISCV_EFI enum entry into two, as is done for other architectures. Reviewed-by: Daniel Kiper 2021-03-02 Marco A Benatto kern/mm: Fix grub_debug_calloc() compilation error Fix compilation error due to missing parameter to grub_printf() when MM_DEBUG is defined. Fixes: 64e26162e (calloc: Make sure we always have an overflow-checking calloc() available) Reviewed-by: Daniel Kiper 2021-03-02 Alex Burmashev templates: Disable the os-prober by default The os-prober is enabled by default what may lead to potentially dangerous use cases and borderline opening attack vectors. This patch disables the os-prober, adds warning messages and updates GRUB_DISABLE_OS_PROBER configuration option documentation. This way we make it clear that the os-prober usage is not recommended. Simplistic nature of this change allows downstream vendors, who really want os-prober to be enabled out of the box in their relevant products, easily revert to it's old behavior. Reported-by: NyankoSec (, https://twitter.com/NyankoSec), working with SSD Secure Disclosure Reviewed-by: Daniel Kiper 2021-03-02 Thomas Frauendorfer | Miray Software gfxmenu/gui: Check printf() format in the gui_progress_bar and gui_label The gui_progress_bar and gui_label components can display the timeout value. The format string can be set through a theme file. This patch adds a validation step to the format string. If a user loads a theme file into the GRUB without this patch then a GUI label with the following settings + label { ... id = "__timeout__" text = "%s" } will interpret the current timeout value as string pointer and print the memory at that position on the screen. It is not desired behavior. Reviewed-by: Daniel Kiper 2021-03-02 Thomas Frauendorfer | Miray Software kern/misc: Add function to check printf() format against expected format The grub_printf_fmt_check() function parses the arguments of an untrusted printf() format and an expected printf() format and then compares the arguments counts and arguments types. The arguments count in the untrusted format string must be less or equal to the arguments count in the expected format string and both arguments types must match. To do this the parse_printf_arg_fmt() helper function is extended in the following way: 1. Add a return value to report errors to the grub_printf_fmt_check(). 2. Add the fmt_check argument to enable stricter format verification: - the function expects that arguments definitions are always terminated by a supported conversion specifier. - positional parameters, "$", are not allowed, as they cannot be validated correctly with the current implementation. For example "%s%1$d" would assign the first args entry twice while leaving the second one unchanged. - Return an error if preallocated space in args is too small and allocation fails for the needed size. The grub_printf_fmt_check() should verify all arguments. So, if validation is not possible for any reason it should return an error. This also adds a case entry to handle "%%", which is the escape sequence to print "%" character. 3. Add the max_args argument to check for the maximum allowed arguments count in a printf() string. This should be set to the arguments count of the expected format. Then the parse_printf_arg_fmt() function will return an error if the arguments count is exceeded. The two additional arguments allow us to use parse_printf_arg_fmt() in printf() and grub_printf_fmt_check() calls. When parse_printf_arg_fmt() is used by grub_printf_fmt_check() the function parse user provided untrusted format string too. So, in that case it is better to be too strict than too lenient. Reviewed-by: Daniel Kiper 2021-03-02 Thomas Frauendorfer | Miray Software kern/misc: Add STRING type for internal printf() format handling Set printf() argument type for "%s" to new type STRING. This is in preparation for a follow up patch to compare a printf() format string against an expected printf() format string. For "%s" the corresponding printf() argument is dereferenced as pointer while all other argument types are defined as integer value. However, when validating a printf() format it is necessary to differentiate "%s" from "%p" and other integers. So, let's do that. Reviewed-by: Daniel Kiper 2021-03-02 Thomas Frauendorfer | Miray Software kern/misc: Split parse_printf_args() into format parsing and va_list handling This patch is preparing for a follow up patch which will use the format parsing part to compare the arguments in a printf() format from an external source against a printf() format with expected arguments. Reviewed-by: Daniel Kiper 2021-03-02 Dimitri John Ledkov shim_lock: Only skip loading shim_lock verifier with explicit consent Commit 32ddc42c (efi: Only register shim_lock verifier if shim_lock protocol is found and SB enabled) reintroduced CVE-2020-15705 which previously only existed in the out-of-tree linuxefi patches and was fixed as part of the BootHole patch series. Under Secure Boot enforce loading shim_lock verifier. Allow skipping shim_lock verifier if SecureBoot/MokSBState EFI variables indicate skipping validations, or if GRUB image is built with --disable-shim-lock. Fixes: 132ddc42c (efi: Only register shim_lock verifier if shim_lock protocol is found and SB enabled) Fixes: CVE-2020-15705 Fixes: CVE-2021-3418 Reported-by: Dimitri John Ledkov Reviewed-by: Daniel Kiper 2021-03-02 Dimitri John Ledkov grub-install-common: Add --sbat option Reviewed-by: Daniel Kiper 2021-03-02 Peter Jones util/mkimage: Add an option to import SBAT metadata into a .sbat section Add a --sbat option to the grub-mkimage tool which allows us to import an SBAT metadata formatted as a CSV file into a .sbat section of the EFI binary. Reviewed-by: Daniel Kiper 2021-03-02 Peter Jones util/mkimage: Refactor section setup to use a helper Add a init_pe_section() helper function to setup PE sections. This makes the code simpler and easier to read. Reviewed-by: Daniel Kiper 2021-03-02 Peter Jones util/mkimage: Improve data_size value calculation According to "Microsoft Portable Executable and Common Object File Format Specification", the Optional Header SizeOfInitializedData field contains: Size of the initialized data section, or the sum of all such sections if there are multiple data sections. Make this explicit by adding the GRUB kernel data size to the sum of all the modules sizes. The ALIGN_UP() is not required by the PE spec but do it to avoid alignment issues. Reviewed-by: Daniel Kiper 2021-03-02 Peter Jones util/mkimage: Reorder PE optional header fields set-up This makes the PE32 and PE32+ header fields set-up easier to follow by setting them closer to the initialization of their related sections. Reviewed-by: Daniel Kiper 2021-03-02 Peter Jones util/mkimage: Unify more of the PE32 and PE32+ header set-up There's quite a bit of code duplication in the code that sets the optional header for PE32 and PE32+. The two are very similar with the exception of a few fields that have type grub_uint64_t instead of grub_uint32_t. Factor out the common code and add a PE_OHDR() macro that simplifies the set-up and make the code more readable. Reviewed-by: Daniel Kiper 2021-03-02 Peter Jones util/mkimage: Always use grub_host_to_target32() to initialize PE stack and heap stuff This change does not impact final result of initialization itself. However, it eases PE code unification in subsequent patches. Reviewed-by: Daniel Kiper 2021-03-02 Peter Jones util/mkimage: Use grub_host_to_target32() instead of grub_cpu_to_le32() The latter doesn't take into account the target image endianness. There is a grub_cpu_to_le32_compile_time() but no compile time variant for function grub_host_to_target32(). So, let's keep using the other one for this case. Reviewed-by: Daniel Kiper 2021-03-02 Javier Martinez Canillas util/mkimage: Remove unused code to add BSS section The code is compiled out so there is no reason to keep it. Additionally, don't set bss_size field since we do not add a BSS section. Reviewed-by: Daniel Kiper 2021-03-02 Chris Coulson kern/efi: Add initial stack protector implementation It works only on UEFI platforms but can be quite easily extended to others architectures and platforms if needed. Reviewed-by: Marco A Benatto Reviewed-by: Javier Martinez Canillas 2021-03-02 Chris Coulson kern/parser: Fix a stack buffer overflow grub_parser_split_cmdline() expands variable names present in the supplied command line in to their corresponding variable contents and uses a 1 kiB stack buffer for temporary storage without sufficient bounds checking. If the function is called with a command line that references a variable with a sufficiently large payload, it is possible to overflow the stack buffer via tab completion, corrupt the stack frame and potentially control execution. Fixes: CVE-2020-27749 Reported-by: Chris Coulson Reviewed-by: Daniel Kiper 2021-03-02 Chris Coulson kern/buffer: Add variable sized heap buffer Add a new variable sized heap buffer type (grub_buffer_t) with simple operations for appending data, accessing the data and maintaining a read cursor. Reviewed-by: Daniel Kiper 2021-03-02 Chris Coulson kern/parser: Refactor grub_parser_split_cmdline() cleanup Introduce a common function epilogue used for cleaning up on all return paths, which will simplify additional error handling to be introduced in a subsequent commit. Reviewed-by: Daniel Kiper 2021-03-02 Chris Coulson kern/parser: Introduce terminate_arg() helper process_char() and grub_parser_split_cmdline() use similar code for terminating the most recent argument. Add a helper function for this. Reviewed-by: Daniel Kiper 2021-03-02 Chris Coulson kern/parser: Introduce process_char() helper grub_parser_split_cmdline() iterates over each command line character. In order to add error checking and to simplify the subsequent error handling, split the character processing in to a separate function. Reviewed-by: Daniel Kiper 2021-03-02 Chris Coulson kern/parser: Fix a memory leak The getline() function supplied to grub_parser_split_cmdline() returns a newly allocated buffer and can be called multiple times, but the returned buffer is never freed. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens fs/btrfs: Squash some uninitialized reads We need to check errors before calling into a function that uses the result. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens fs/btrfs: Validate the number of stripes/parities in RAID5/6 This prevents a divide by zero if nstripes == nparities, and also prevents propagation of invalid values if nstripes ends up less than nparities. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens disk/lvm: Do not allow a LV to be it's own segment's node's LV This prevents infinite recursion in the diskfilter verification code. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens disk/lvm: Sanitize rlocn->offset to prevent wild read rlocn->offset is read directly from disk and added to the metadatabuf pointer to create a pointer to a block of metadata. It's a 64-bit quantity so as long as you don't overflow you can set subsequent pointers to point anywhere in memory. Require that rlocn->offset fits within the metadata buffer size. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens disk/lvm: Do not overread metadata We could reach the end of valid metadata and not realize, leading to some buffer overreads. Check if we have reached the end and bail. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens disk/lvm: Do not crash if an expected string is not found Clean up a bunch of cases where we could have strstr() fail and lead to us dereferencing NULL. We'll still leak memory in some cases (loops don't clean up allocations from earlier iterations if a later iteration fails) but at least we're not crashing. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens disk/lvm: Bail on missing PV list There's an if block for the presence of "physical_volumes {", but if that block is absent, then p remains NULL and a NULL-deref will result when looking for logical volumes. It doesn't seem like LVM makes sense without physical volumes, so error out rather than crashing. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens disk/lvm: Don't blast past the end of the circular metadata buffer This catches at least some OOB reads, and it's possible I suppose that if 2 * mda_size is less than GRUB_LVM_MDA_HEADER_SIZE it might catch some OOB writes too (although that hasn't showed up as a crash in fuzzing yet). It's a bit ugly and I'd appreciate better suggestions. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens disk/lvm: Don't go beyond the end of the data we read from disk We unconditionally trusted offset_xl from the LVM label header, even if it told us that the PV header/disk locations were way off past the end of the data we read from disk. Require that the offset be sane, fixing an OOB read and crash. Fixes: CID 314367, CID 314371 Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens io/gzio: Zero gzio->tl/td in init_dynamic_block() if huft_build() fails If huft_build() fails, gzio->tl or gzio->td could contain pointers that are no longer valid. Zero them out. This prevents a double free when grub_gzio_close() comes through and attempts to free them again. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens io/gzio: Catch missing values in huft_build() and bail In huft_build(), "v" is a table of values in order of bit length. The code later (when setting up table entries in "r") assumes that all elements of this array corresponding to a code are initialized and less than N_MAX. However, it doesn't enforce this. With sufficiently manipulated inputs (e.g. from fuzzing), there can be elements of "v" that are not filled. Therefore a lookup into "e" or "d" will use an uninitialized value. This can lead to an invalid/OOB read on those values, often leading to a crash. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens io/gzio: Add init_dynamic_block() clean up if unpacking codes fails init_dynamic_block() didn't clean up gzio->tl and td in some error paths. This left td pointing to part of tl. Then in grub_gzio_close(), when tl was freed the storage for td would also be freed. The code then attempts to free td explicitly, performing a UAF and then a double free. Explicitly clean up tl and td in the error paths. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens io/gzio: Bail if gzio->tl/td is NULL This is an ugly fix that doesn't address why gzio->tl comes to be NULL. However, it seems to be sufficient to patch up a bunch of NULL derefs. It would be good to revisit this in future and see if we can have a cleaner solution that addresses some of the causes of the unexpected NULL pointers. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens fs/nilfs2: Properly bail on errors in grub_nilfs2_btree_node_lookup() We just introduced an error return in grub_nilfs2_btree_node_lookup(). Make sure the callers catch it. At the same time, make sure that grub_nilfs2_btree_node_lookup() always inits the index pointer passed to it. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens fs/nilfs2: Don't search children if provided number is too large NILFS2 reads the number of children a node has from the node. Unfortunately, that's not trustworthy. Check if it's beyond what the filesystem permits and reject it if so. This blocks some OOB reads. I'm not sure how controllable the read is and what could be done with invalidly read data later on. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens fs/nilfs2: Reject too-large keys NILFS2 has up to 7 keys, per the data structure. Do not permit array indices in excess of that. This catches some OOB reads. I don't know how controllable the invalidly read data is or if that could be used later in the program. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens fs/jfs: Catch infinite recursion It's possible with a fuzzed filesystem for JFS to keep getblk()-ing the same data over and over again, leading to stack exhaustion. Check if we'd be calling the function with exactly the same data as was passed in, and if so abort. I'm not sure what the performance impact of this is and am open to better ideas. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens fs/jfs: Limit the extents that getblk() can consider getblk() implicitly trusts that treehead->count is an accurate count of the number of extents. However, that value is read from disk and is not trustworthy, leading to OOB reads and crashes. I am not sure to what extent the data read from OOB can influence subsequent program execution. Require callers to pass in the maximum number of extents for which they have storage. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens fs/jfs: Do not move to leaf level if name length is negative Fuzzing JFS revealed crashes where a negative number would be passed to le_to_cpu16_copy(). There it would be cast to a large positive number and the copy would read and write off the end of the respective buffers. Catch this at the top as well as the bottom of the loop. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens fs/sfs: Fix over-read of root object name There's a read of the name of the root object that assumes that the name is nul-terminated within the root block. This isn't guaranteed - it seems SFS would require you to read multiple blocks to get a full name in general, but maybe that doesn't apply to the root object. Either way, figure out how much space is left in the root block and don't over-read it. This fixes some OOB reads. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens fs/hfs: Disable under lockdown HFS has issues such as infinite mutual recursion that are simply too complex to fix for such a legacy format. So simply do not permit it to be loaded under lockdown. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens fs/hfsplus: Don't use uninitialized data on corrupt filesystems Valgrind identified the following use of uninitialized data: ==2782220== Conditional jump or move depends on uninitialised value(s) ==2782220== at 0x42B364: grub_hfsplus_btree_search (hfsplus.c:566) ==2782220== by 0x42B21D: grub_hfsplus_read_block (hfsplus.c:185) ==2782220== by 0x42A693: grub_fshelp_read_file (fshelp.c:386) ==2782220== by 0x42C598: grub_hfsplus_read_file (hfsplus.c:219) ==2782220== by 0x42C598: grub_hfsplus_mount (hfsplus.c:330) ==2782220== by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958) ==2782220== by 0x4C1AE6: grub_fs_probe (fs.c:73) ==2782220== by 0x407C94: grub_ls_list_files (ls.c:186) ==2782220== by 0x407C94: grub_cmd_ls (ls.c:284) ==2782220== by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55) ==2782220== by 0x4045A6: execute_command (grub-fstest.c:59) ==2782220== by 0x4045A6: fstest (grub-fstest.c:433) ==2782220== by 0x4045A6: main (grub-fstest.c:772) ==2782220== Uninitialised value was created by a heap allocation ==2782220== at 0x483C7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==2782220== by 0x4C0305: grub_malloc (mm.c:42) ==2782220== by 0x42C21D: grub_hfsplus_mount (hfsplus.c:239) ==2782220== by 0x42B8C5: grub_hfsplus_dir (hfsplus.c:958) ==2782220== by 0x4C1AE6: grub_fs_probe (fs.c:73) ==2782220== by 0x407C94: grub_ls_list_files (ls.c:186) ==2782220== by 0x407C94: grub_cmd_ls (ls.c:284) ==2782220== by 0x4D7130: grub_extcmd_dispatcher (extcmd.c:55) ==2782220== by 0x4045A6: execute_command (grub-fstest.c:59) ==2782220== by 0x4045A6: fstest (grub-fstest.c:433) ==2782220== by 0x4045A6: main (grub-fstest.c:772) This happens when the process of reading the catalog file goes sufficiently wrong that there's an attempt to read the extent overflow file, which has not yet been loaded. Keep track of when the extent overflow file is fully loaded and refuse to use it before then. The load valgrind doesn't like is btree->nodesize, and that's then used to allocate a data structure. It looks like there are subsequently a lot of reads based on that pointer so OOB reads are likely, and indeed crashes (albeit difficult-to-replicate ones) have been observed in fuzzing. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens fs/hfsplus: Don't fetch a key beyond the end of the node Otherwise you get a wild pointer, leading to a bunch of invalid reads. Check it falls inside the given node. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens fs/fshelp: Catch impermissibly large block sizes in read helper A fuzzed HFS+ filesystem had log2blocksize = 22. This gave log2blocksize + GRUB_DISK_SECTOR_BITS = 31. 1 << 31 = 0x80000000, which is -1 as an int. This caused some wacky behavior later on in the function, leading to out-of-bounds writes on the destination buffer. Catch log2blocksize + GRUB_DISK_SECTOR_BITS >= 31. We could be stricter, but this is the minimum that will prevent integer size weirdness. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens term/gfxterm: Don't set up a font with glyphs that are too big Catch the case where we have a font so big that it causes the number of rows or columns to be 0. Currently we continue and allocate a virtual_screen.text_buffer of size 0. We then try to use that for glpyhs and things go badly. On the emu platform, malloc() may give us a valid pointer, in which case we'll access heap memory which we shouldn't. Alternatively, it may give us NULL, in which case we'll crash. For other platforms, if I understand grub_memalign() correctly, we will receive a valid but small allocation that we will very likely later overrun. Prevent the creation of a virtual screen that isn't at least 40 cols by 12 rows. This is arbitrary, but it seems that if your width or height is half a standard 80x24 terminal, you're probably going to struggle to read anything anyway. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens video/readers/jpeg: Don't decode data before start of stream When a start of stream marker is encountered, we call grub_jpeg_decode_sos() which allocates space for a bitmap. When a restart marker is encountered, we call grub_jpeg_decode_data() which then fills in that bitmap. If we get a restart marker before the start of stream marker, we will attempt to write to a bitmap_ptr that hasn't been allocated. Catch this and bail out. This fixes an attempt to write to NULL. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens video/readers/jpeg: Catch OOB reads/writes in grub_jpeg_decode_du() The key line is: du[jpeg_zigzag_order[pos]] = val * (int) data->quan_table[qt][pos]; jpeg_zigzag_order is grub_uint8_t[64]. I don't understand JPEG decoders quite well enough to explain what's going on here. However, I observe sometimes pos=64, which leads to an OOB read of the jpeg_zigzag_order global then an OOB write to du. That leads to various unpleasant memory corruption conditions. Catch where pos >= ARRAY_SIZE(jpeg_zigzag_order) and bail. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens video/readers/jpeg: Catch files with unsupported quantization or Huffman tables Our decoder only supports 2 quantization tables. If a file asks for a quantization table with index > 1, reject it. Similarly, our decoder only supports 4 Huffman tables. If a file asks for a Huffman table with index > 3, reject it. This fixes some out of bounds reads. It's not clear what degree of control over subsequent execution could be gained by someone who can carefully set up the contents of memory before loading an invalid JPEG file. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens kern/misc: Always set *end in grub_strtoull() Currently, if there is an error in grub_strtoull(), *end is not set. This differs from the usual behavior of strtoull(), and also means that some callers may use an uninitialized value for *end. Set *end unconditionally. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens commands/menuentry: Fix quoting in setparams_prefix() Commit 9acdcbf32542 (use single quotes in menuentry setparams command) says that expressing a quoted single quote will require 3 characters. It actually requires (and always did require!) 4 characters: str: a'b => a'\''b len: 3 => 6 (2 for the letters + 4 for the quote) This leads to not allocating enough memory and thus out of bounds writes that have been observed to cause heap corruption. Allocate 4 bytes for each single quote. Commit 22e7dbb2bb81 (Fix quoting in legacy parser.) does the same quoting, but it adds 3 as extra overhead on top of the single byte that the quote already needs. So it's correct. Fixes: 9acdcbf32542 (use single quotes in menuentry setparams command) Fixes: CVE-2021-20233 Reported-by: Daniel Axtens Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens script/execute: Don't crash on a "for" loop with no items The following crashes the parser: for x in; do 0 done This is because grub_script_arglist_to_argv() doesn't consider the possibility that arglist is NULL. Catch that explicitly. This avoids a NULL pointer dereference. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens lib/arg: Block repeated short options that require an argument Fuzzing found the following crash: search -hhhhhhhhhhhhhf We didn't allocate enough option space for 13 hints because the allocation code counts the number of discrete arguments (i.e. argc). However, the shortopt parsing code will happily keep processing a combination of short options without checking if those short options require an argument. This means you can easily end writing past the allocated option space. This fixes a OOB write which can cause heap corruption. Fixes: CVE-2021-20225 Reported-by: Daniel Axtens Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens script/execute: Avoid crash when using "$#" outside a function scope "$#" represents the number of arguments to a function. It is only defined in a function scope, where "scope" is non-NULL. Currently, if we attempt to evaluate "$#" outside a function scope, "scope" will be NULL and we will crash with a NULL pointer dereference. Do not attempt to count arguments for "$#" if "scope" is NULL. This will result in "$#" being interpreted as an empty string if evaluated outside a function scope. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens commands/ls: Require device_name is not NULL before printing This can be triggered with: ls -l (0 0*) and causes a NULL deref in grub_normal_print_device_info(). I'm not sure if there's any implication with the IEEE 1275 platform. Reviewed-by: Daniel Kiper 2021-03-02 Daniel Axtens script/execute: Fix NULL dereference in grub_script_execute_cmdline() Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny util/glue-efi: Fix incorrect use of a possibly negative value It is possible for the ftell() function to return a negative value, although it is fairly unlikely here, we should be checking for a negative value before we assign it to an unsigned value. Fixes: CID 73744 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny util/grub-editenv: Fix incorrect casting of a signed value The return value of ftell() may be negative (-1) on error. While it is probably unlikely to occur, we should not blindly cast to an unsigned value without first testing that it is not negative. Fixes: CID 73856 Reviewed-by: Daniel Kiper 2021-03-02 Daniel Kiper util/grub-install: Fix NULL pointer dereferences Two grub_device_open() calls does not have associated NULL checks for returned values. Fix that and appease the Coverity. Fixes: CID 314583 Reviewed-by: Javier Martinez Canillas 2021-03-02 Paulo Flabiano Smorigo loader/xnu: Check if pointer is NULL before using it Fixes: CID 73654 Reviewed-by: Daniel Kiper 2021-03-02 Marco A Benatto loader/xnu: Free driverkey data when an error is detected in grub_xnu_writetree_toheap() ... to avoid memory leaks. Fixes: CID 96640 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny loader/xnu: Fix memory leak The code here is finished with the memory stored in name, but it only frees it if there curvalue is valid, while it could actually free it regardless. The fix is a simple relocation of the grub_free() to before the test of curvalue. Fixes: CID 96646 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny loader/bsd: Check for NULL arg up-front The code in the next block suggests that it is possible for .set to be true but .arg may still be NULL. This code assumes that it is never NULL, yet later is testing if it is NULL - that is inconsistent. So we should check first if .arg is not NULL, and remove this check that is being flagged by Coverity since it is no longer required. Fixes: CID 292471 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny gfxmenu/gui_list: Remove code that coverity is flagging as dead The test of value for NULL before calling grub_strdup() is not required, since the if condition prior to this has already tested for value being NULL and cannot reach this code if it is. Fixes: CID 73659 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny video/readers/jpeg: Test for an invalid next marker reference from a jpeg file While it may never happen, and potentially could be caught at the end of the function, it is worth checking up front for a bad reference to the next marker just in case of a maliciously crafted file being provided. Fixes: CID 73694 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny video/fb/video_fb: Fix possible integer overflow It is minimal possibility that the values being used here will overflow. So, change the code to use the safemath function grub_mul() to ensure that doesn't happen. Fixes: CID 73761 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny video/fb/video_fb: Fix multiple integer overflows The calculation of the unsigned 64-bit value is being generated by multiplying 2, signed or unsigned, 32-bit integers which may overflow before promotion to unsigned 64-bit. Fix all of them. Fixes: CID 73703, CID 73767, CID 73833 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny video/fb/fbfill: Fix potential integer overflow The multiplication of 2 unsigned 32-bit integers may overflow before promotion to unsigned 64-bit. We should ensure that the multiplication is done with overflow detection. Additionally, use grub_sub() for subtraction. Fixes: CID 73640, CID 73697, CID 73702, CID 73823 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny video/efi_gop: Remove unnecessary return value of grub_video_gop_fill_mode_info() The return value of grub_video_gop_fill_mode_info() is never able to be anything other than GRUB_ERR_NONE. So, rather than continue to return a value and checking it each time, it is more correct to redefine the function to not return anything and remove checks of its return value altogether. Fixes: CID 96701 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny commands/probe: Fix a resource leak when probing disks Every other return statement in this code is calling grub_device_close() to clean up dev before returning. This one should do that too. Fixes: CID 292443 Reviewed-by: Daniel Kiper 2021-03-02 Chris Coulson commands/hashsum: Fix a memory leak check_list() uses grub_file_getline(), which allocates a buffer. If the hash list file contains invalid lines, the function leaks this buffer when it returns an error. Fixes: CID 176635 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny normal/completion: Fix leaking of memory when processing a completion It is possible for the code to reach the end of the function without freeing the memory allocated to argv and argc still to be 0. We should always call grub_free(argv). The grub_free() will handle a NULL argument correctly if it reaches that code without the memory being allocated. Fixes: CID 96672 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny syslinux: Fix memory leak while parsing In syslinux_parse_real() the 2 points where return is being called didn't release the memory stored in buf which is no longer required. Fixes: CID 176634 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny libgcrypt/mpi: Fix possible NULL dereference The code in gcry_mpi_scan() assumes that buffer is not NULL, but there is no explicit check for that, so we add one. Fixes: CID 73757 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny libgcrypt/mpi: Fix possible unintended sign extension The array of unsigned char gets promoted to a signed 32-bit int before it is finally promoted to a size_t. There is the possibility that this may result in the signed-bit being set for the intermediate signed 32-bit int. We should ensure that the promotion is to the correct type before we bitwise-OR the values. Fixes: CID 96697 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny affs: Fix memory leaks The node structure reference is being allocated but not freed if it reaches the end of the function. If any of the hooks had returned a non-zero value, then node would have been copied in to the context reference, but otherwise node is not stored and should be freed. Similarly, the call to grub_affs_create_node() replaces the allocated memory in node with a newly allocated structure, leaking the existing memory pointed by node. Finally, when dir->parent is set, then we again replace node with newly allocated memory, which seems unnecessary when we copy in the values from dir->parent immediately after. Fixes: CID 73759 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny zfsinfo: Correct a check for error allocating memory While arguably the check for grub_errno is correct, we should really be checking the return value from the function since it is always possible that grub_errno was set elsewhere, making this code behave incorrectly. Fixes: CID 73668 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny zfs: Fix possible integer overflows In all cases the problem is that the value being acted upon by a left-shift is a 32-bit number which is then being used in the context of a 64-bit number. To avoid overflow we ensure that the number being shifted is 64-bit before the shift is done. Fixes: CID 73684, CID 73695, CID 73764 Reviewed-by: Daniel Kiper 2021-03-02 Paulo Flabiano Smorigo zfs: Fix resource leaks while constructing path There are several exit points in dnode_get_path() that are causing possible memory leaks. In the while(1) the correct exit mechanism should not be to do a direct return, but to instead break out of the loop, setting err first if it is not already set. The reason behind this is that the dnode_path is a linked list, and while doing through this loop, it is being allocated and built up - the only way to correctly unravel it is to traverse it, which is what is being done at the end of the function outside of the loop. Several of the existing exit points correctly did a break, but not all so this change makes that more consistent and should resolve the leaking of memory as found by Coverity. Fixes: CID 73741 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny zfs: Fix possible negative shift operation While it is possible for the return value from zfs_log2() to be zero (0), it is quite unlikely, given that the previous assignment to blksz is shifted up by SPA_MINBLOCKSHIFT (9) before 9 is subtracted at the assignment to epbs. But, while unlikely during a normal operation, it may be that a carefully crafted ZFS filesystem could result in a zero (0) value to the dn_datalbkszsec field, which means that the shift left does nothing and assigns zero (0) to blksz, resulting in a negative epbs value. Fixes: CID 73608 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny hfsplus: Check that the volume name length is valid HFS+ documentation suggests that the maximum filename and volume name is 255 Unicode characters in length. So, when converting from big-endian to little-endian, we should ensure that the name of the volume has a length that is between 0 and 255, inclusive. Fixes: CID 73641 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny disk/cryptodisk: Fix potential integer overflow The encrypt and decrypt functions expect a grub_size_t. So, we need to ensure that the constant bit shift is using grub_size_t rather than unsigned int when it is performing the shift. Fixes: CID 307788 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny disk/ldm: Fix memory leak on uninserted lv references The problem here is that the memory allocated to the variable lv is not yet inserted into the list that is being processed at the label fail2. As we can already see at line 342, which correctly frees lv before going to fail2, we should also be doing that at these earlier jumps to fail2. Fixes: CID 73824 Reviewed-by: Daniel Kiper 2021-03-02 Paulo Flabiano Smorigo disk/ldm: If failed then free vg variable too Fixes: CID 73809 Reviewed-by: Daniel Kiper 2021-03-02 Marco A Benatto disk/ldm: Make sure comp data is freed before exiting from make_vg() Several error handling paths in make_vg() do not free comp data before jumping to fail2 label and returning from the function. This will leak memory. So, let's fix all issues of that kind. Fixes: CID 73804 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny kern/partition: Check for NULL before dereferencing input string There is the possibility that the value of str comes from an external source and continuing to use it before ever checking its validity is wrong. So, needs fixing. Additionally, drop unneeded part initialization. Fixes: CID 292444 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny zstd: Initialize seq_t structure fully While many compilers will initialize this to zero, not all will, so it is better to be sure that fields not being explicitly set are at known values, and there is code that checks this fields value elsewhere in the code. Fixes: CID 292440 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny io/lzopio: Resolve unnecessary self-assignment errors These 2 assignments are unnecessary since they are just assigning to themselves. Fixes: CID 73643 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny gnulib/regcomp: Fix uninitialized re_token This issue has been fixed in the latest version of gnulib, so to maintain consistency, I've backported that change rather than doing something different. Fixes: CID 73828 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny gnulib/regexec: Fix possible null-dereference It appears to be possible that the mctx->state_log field may be NULL, and the name of this function, clean_state_log_if_needed(), suggests that it should be checking that it is valid to be cleaned before assuming that it does. Fixes: CID 86720 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny gnulib/argp-help: Fix dereference of a possibly NULL state All other instances of call to __argp_failure() where there is a dgettext() call is first checking whether state is NULL before attempting to dereference it to get the root_argp->argp_domain. Fixes: CID 292436 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny gnulib/regcomp: Fix uninitialized token structure The code is assuming that the value of br_token.constraint was initialized to zero when it wasn't. While some compilers will ensure that, not all do, so it is better to fix this explicitly than leave it to chance. Fixes: CID 73749 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny gnulib/regexec: Resolve unused variable This is a really minor issue where a variable is being assigned to but not checked before it is overwritten again. The reason for this issue is that we are not building with DEBUG set and this in turn means that the assert() that reads the value of the variable match_last is being processed out. The solution, move the assignment to match_last in to an ifdef DEBUG too. Fixes: CID 292459 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny kern/efi/mm: Fix possible NULL pointer dereference The model of grub_efi_get_memory_map() is that if memory_map is NULL, then the purpose is to discover how much memory should be allocated to it for the subsequent call. The problem here is that with grub_efi_is_finished set to 1, there is no check at all that the function is being called with a non-NULL memory_map. While this MAY be true, we shouldn't assume it. The solution to this is to behave as expected, and if memory_map is NULL, then don't try to use it and allow memory_map_size to be filled in, and return 0 as is done later in the code if the buffer is too small (or NULL). Additionally, drop unneeded ret = 1. Fixes: CID 96632 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny kern/efi: Fix memory leak on failure Free the memory allocated to name before returning on failure. Fixes: CID 296222 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny kern/parser: Fix resource leak if argc == 0 After processing the command-line yet arriving at the point where we are setting argv, we are allocating memory, even if argc == 0, which makes no sense since we never put anything into the allocated argv. The solution is to simply return that we've successfully processed the arguments but that argc == 0, and also ensure that argv is NULL when we're not allocating anything in it. There are only 2 callers of this function, and both are handling a zero value in argc assuming nothing is allocated in argv. Fixes: CID 96680 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny net/tftp: Fix dangling memory pointer The static code analysis tool, Parfait, reported that the valid of file->data was left referencing memory that was freed by the call to grub_free(data) where data was initialized from file->data. To ensure that there is no unintentional access to this memory referenced by file->data we should set the pointer to NULL. Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny net/net: Fix possible dereference to of a NULL pointer It is always possible that grub_zalloc() could fail, so we should check for a NULL return. Otherwise we run the risk of dereferencing a NULL pointer. Fixes: CID 296221 Reviewed-by: Daniel Kiper 2021-03-02 Darren Kenny mmap: Fix memory leak when iterating over mapped memory When returning from grub_mmap_iterate() the memory allocated to present is not being released causing it to leak. Fixes: CID 96655 Reviewed-by: Daniel Kiper 2021-03-02 Javier Martinez Canillas usb: Avoid possible out-of-bound accesses caused by malicious devices The maximum number of configurations and interfaces are fixed but there is no out-of-bound checking to prevent a malicious USB device to report large values for these and cause accesses outside the arrays' memory. Fixes: CVE-2020-25647 Reported-by: Joseph Tartaro Reported-by: Ilja Van Sprundel Reviewed-by: Daniel Kiper 2021-03-02 Javier Martinez Canillas dl: Only allow unloading modules that are not dependencies When a module is attempted to be removed its reference counter is always decremented. This means that repeated rmmod invocations will cause the module to be unloaded even if another module depends on it. This may lead to a use-after-free scenario allowing an attacker to execute arbitrary code and by-pass the UEFI Secure Boot protection. While being there, add the extern keyword to some function declarations in that header file. Fixes: CVE-2020-25632 Reported-by: Chris Coulson Reviewed-by: Daniel Kiper 2021-03-02 Javier Martinez Canillas docs: Document the cutmem command The command is not present in the docs/grub.texi user documentation. Reported-by: Daniel Kiper Reviewed-by: Javier Martinez Canillas 2021-03-02 Javier Martinez Canillas loader/xnu: Don't allow loading extension and packages when locked down The shim_lock verifier validates the XNU kernels but no its extensions and packages. Prevent these to be loaded when the GRUB is locked down. Reviewed-by: Daniel Kiper 2021-03-02 Javier Martinez Canillas gdb: Restrict GDB access when locked down The gdbstub* commands allow to start and control a GDB stub running on local host that can be used to connect from a remote debugger. Restrict this functionality when the GRUB is locked down. Reviewed-by: Daniel Kiper 2021-03-02 Javier Martinez Canillas commands/hdparm: Restrict hdparm command when locked down The command can be used to get/set ATA disk parameters. Some of these can be dangerous since change the disk behavior. Restrict it when locked down. Reviewed-by: Daniel Kiper 2021-03-02 Javier Martinez Canillas commands/setpci: Restrict setpci command when locked down This command can set PCI devices register values, which makes it dangerous in a locked down configuration. Restrict it so can't be used on this setup. Reviewed-by: Daniel Kiper 2021-03-02 Javier Martinez Canillas commands: Restrict commands that can load BIOS or DT blobs when locked down There are some more commands that should be restricted when the GRUB is locked down. Following is the list of commands and reasons to restrict: * fakebios: creates BIOS-like structures for backward compatibility with existing OSes. This should not be allowed when locked down. * loadbios: reads a BIOS dump from storage and loads it. This action should not be allowed when locked down. * devicetree: loads a Device Tree blob and passes it to the OS. It replaces any Device Tree provided by the firmware. This also should not be allowed when locked down. Reviewed-by: Daniel Kiper 2021-03-02 Javier Martinez Canillas mmap: Don't register cutmem and badram commands when lockdown is enforced The cutmem and badram commands can be used to remove EFI memory regions and potentially disable the UEFI Secure Boot. Prevent the commands to be registered if the GRUB is locked down. Fixes: CVE-2020-27779 Reported-by: Teddy Reed Reviewed-by: Daniel Kiper 2021-03-02 Javier Martinez Canillas acpi: Don't register the acpi command when locked down The command is not allowed when lockdown is enforced. Otherwise an attacker can instruct the GRUB to load an SSDT table to overwrite the kernel lockdown configuration and later load and execute unsigned code. Fixes: CVE-2020-14372 Reported-by: Máté Kukri Reviewed-by: Daniel Kiper 2021-03-02 Javier Martinez Canillas efi: Use grub_is_lockdown() instead of hardcoding a disabled modules list Now the GRUB can check if it has been locked down and this can be used to prevent executing commands that can be utilized to circumvent the UEFI Secure Boot mechanisms. So, instead of hardcoding a list of modules that have to be disabled, prevent the usage of commands that can be dangerous. This not only allows the commands to be disabled on other platforms, but also properly separate the concerns. Since the shim_lock verifier logic should be only about preventing to run untrusted binaries and not about defining these kind of policies. Reviewed-by: Daniel Kiper 2021-03-02 Javier Martinez Canillas efi: Lockdown the GRUB when the UEFI Secure Boot is enabled If the UEFI Secure Boot is enabled then the GRUB must be locked down to prevent executing code that can potentially be used to subvert its verification mechanisms. Reviewed-by: Daniel Kiper 2021-03-02 Javier Martinez Canillas kern/lockdown: Set a variable if the GRUB is locked down It may be useful for scripts to determine whether the GRUB is locked down or not. Add the lockdown variable which is set to "y" when the GRUB is locked down. Suggested-by: Dimitri John Ledkov Reviewed-by: Daniel Kiper 2021-03-02 Javier Martinez Canillas kern: Add lockdown support When the GRUB starts on a secure boot platform, some commands can be used to subvert the protections provided by the verification mechanism and could lead to booting untrusted system. To prevent that situation, allow GRUB to be locked down. That way the code may check if GRUB has been locked down and further restrict the commands that are registered or what subset of their functionality could be used. The lockdown support adds the following components: * The grub_lockdown() function which can be used to lockdown GRUB if, e.g., UEFI Secure Boot is enabled. * The grub_is_lockdown() function which can be used to check if the GRUB was locked down. * A verifier that flags OS kernels, the GRUB modules, Device Trees and ACPI tables as GRUB_VERIFY_FLAGS_DEFER_AUTH to defer verification to other verifiers. These files are only successfully verified if another registered verifier returns success. Otherwise, the whole verification process fails. For example, PE/COFF binaries verification can be done by the shim_lock verifier which validates the signatures using the shim_lock protocol. However, the verification is not deferred directly to the shim_lock verifier. The shim_lock verifier is hooked into the verification process instead. * A set of grub_{command,extcmd}_lockdown functions that can be used by code registering command handlers, to only register unsafe commands if the GRUB has not been locked down. Reviewed-by: Daniel Kiper 2021-03-02 Marco A Benatto efi: Move the shim_lock verifier to the GRUB core Move the shim_lock verifier from its own module into the core image. The Secure Boot lockdown mechanism has the intent to prevent the load of any unsigned code or binary when Secure Boot is enabled. The reason is that GRUB must be able to prevent executing untrusted code if UEFI Secure Boot is enabled, without depending on external modules. Reviewed-by: Daniel Kiper 2021-03-02 Marco A Benatto verifiers: Move verifiers API to kernel image Move verifiers API from a module to the kernel image, so it can be used there as well. There are no functional changes in this patch. Reviewed-by: Daniel Kiper 2020-12-18 Glenn Washburn docs: Add documentation of disk size limitations Document the artificially imposed 1 EiB disk size limit and size limitations with LUKS volumes. Fix a few punctuation issues. Reviewed-by: Daniel Kiper 2020-12-18 Glenn Washburn luks2: Use grub_log2ull() to calculate log_sector_size and improve readability Reviewed-by: Daniel Kiper misc: Add grub_log2ull() macro for calculating log base 2 of 64-bit integers Reviewed-by: Daniel Kiper 2020-12-18 Glenn Washburn mips: Enable __clzdi2() This patch is similar to commit 9dab2f51e (sparc: Enable __clzsi2() and __clzdi2()) but for MIPS target and __clzdi2() only, __clzsi2() was already enabled. Suggested-by: Daniel Kiper Reviewed-by: Daniel Kiper 2020-12-18 Glenn Washburn luks2: Better error handling when setting up the cryptodisk Do some sanity checking on data coming from the LUKS2 header. If segment.size is "dynamic", verify that the offset is not past the end of disk. Otherwise, check for errors from grub_strtoull() when converting segment size from string. If a GRUB_ERR_BAD_NUMBER error was returned, then the string was not a valid parsable number, so skip the key. If GRUB_ERR_OUT_OF_RANGE was returned, then there was an overflow in converting to a 64-bit unsigned integer. So this could be a very large disk (perhaps large RAID array). In this case skip the key too. Additionally, enforce some other limits and fail if needed. Reviewed-by: Daniel Kiper 2020-12-18 Glenn Washburn luks2: Do not handle disks of size GRUB_DISK_SIZE_UNKNOWN for now Check to make sure that source disk has a known size. If not, print a message and return error. There are 4 cases where GRUB_DISK_SIZE_UNKNOWN is set (biosdisk, obdisk, ofdisk, and uboot), and in all those cases processing continues. So this is probably a bit conservative. However, 3 of the cases seem pathological, and the other, biosdisk, happens when booting from a CD-ROM. Since I doubt booting from a LUKS2 volume on a CD-ROM is a big use case, we'll error until someone complains. Reviewed-by: Daniel Kiper 2020-12-18 Glenn Washburn luks2: Convert to crypt sectors from GRUB native sectors The function grub_disk_native_sectors(source) returns the number of sectors of source in GRUB native (512-byte) sectors, not source sized sectors. So the conversion needs to use GRUB_DISK_SECTOR_BITS, the GRUB native sector size. Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn luks2: Error check segment.sector_size Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn cryptodisk: Properly handle non-512 byte sized sectors By default, dm-crypt internally uses an IV that corresponds to 512-byte sectors, even when a larger sector size is specified. What this means is that when using a larger sector size, the IV is incremented every sector. However, the amount the IV is incremented is the number of 512 byte blocks in a sector (i.e. 8 for 4K sectors). Confusingly the IV does not correspond to the number of, for example, 4K sectors. So each 512 byte cipher block in a sector will be encrypted with the same IV and the IV will be incremented afterwards by the number of 512 byte cipher blocks in the sector. There are some encryption utilities which do it the intuitive way and have the IV equal to the sector number regardless of sector size (ie. the fifth sector would have an IV of 4 for each cipher block). And this is supported by dm-crypt with the iv_large_sectors option and also cryptsetup as of 2.3.3 with the --iv-large-sectors, though not with LUKS headers (only with --type plain). However, support for this has not been included as grub does not support plain devices right now. One gotcha here is that the encrypted split keys are encrypted with a hard- coded 512-byte sector size. So even if your data is encrypted with 4K sector sizes, the split key encrypted area must be decrypted with a block size of 512 (ie the IV increments every 512 bytes). This made these changes less aesthetically pleasing than desired. Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn luks2: grub_cryptodisk_t->total_sectors is the max number of device native sectors We need to convert the sectors from the size of the underlying device to the cryptodisk sector size; segment.size is in bytes which need to be converted to cryptodisk sectors as well. Also, removed an empty statement. Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn cryptodisk: Add macros GRUB_TYPE_U_MAX/MIN(type) to replace literals Add GRUB_TYPE_U_MAX/MIN(type) macros to get the max/min values for an unsigned number with size of type. Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn cryptodisk: Add macro GRUB_TYPE_BITS() to replace some literals The new macro GRUB_TYPE_BITS(type) returns the number of bits allocated for type. Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn luks2: Add string "index" to user strings using a json index This allows error messages to be more easily distinguishable between indexes and slot keys. The former include the string "index" in the error/debug string, and the later are surrounded in quotes. Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn luks2: Rename json index variables to names that they are obviously json indexes Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn luks2: Use more intuitive object name instead of json index in user messages Use the object name in the json array rather than the 0 based index in the json array for keyslots, segments, and digests. This is less confusing for the end user. For example, say you have a LUKS2 device with a key in slot 1 and slot 4. When using the password for slot 4 to unlock the device, the messages using the index of the keyslot will mention keyslot 1 (its a zero-based index). Furthermore, with this change the keyslot number will align with the number used to reference the keyslot when using the --key-slot argument to cryptsetup. Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn luks2: Add idx member to struct grub_luks2_keyslot/segment/digest This allows code using these structs to know the named key associated with these json data structures. In the future we can use these to provide better error messages to the user. Get rid of idx local variable in luks2_get_keyslot() which was overloaded to be used for both keyslot and segment slot keys. Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn luks2: Make sure all fields of output argument in luks2_parse_digest() are written to We should assume that the output argument "out" is uninitialized and could have random data. So, make sure to initialize the segments and keyslots bit fields because potentially not all bits of those fields are written to. Otherwise, the digest could say it belongs to keyslots and segments that it does not. Reviewed-by: Patrick Steinhardt Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn luks2: Remove unused argument in grub_error() call Reviewed-by: Patrick Steinhardt Reviewed-by: Daniel Kiper luks2: Convert 8 spaces to tabs Reviewed-by: Patrick Steinhardt Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn misc: Add parentheses around ALIGN_UP() and ALIGN_DOWN() arguments This ensures that expected order of operations is preserved when arguments are expressions. Reviewed-by: Patrick Steinhardt Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn 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 Reviewed-by: Patrick Steinhardt Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn 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. Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn 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. Reviewed-by: Daniel Kiper 2020-12-12 Glenn Washburn 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. Reviewed-by: Daniel Kiper 2020-12-12 Vladimir Serbinenko mbr: Document new limitations on MBR gap support Reviewed-by: Daniel Kiper 2020-12-12 Vladimir Serbinenko 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. Reviewed-by: Daniel Kiper 2020-12-12 Tianjia Zhang 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. Reviewed-by: Daniel Kiper 2020-12-12 Tianjia Zhang 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. Reviewed-by: Daniel Kiper 2020-12-12 Daniel Kiper 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. Reviewed-by: Daniel Kiper 2020-12-12 Javier Martinez Canillas 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 Reported-by: Peter Jones Reviewed-by: Daniel Kiper 2020-12-11 Daniel Kiper efi: Add secure boot detection Introduce grub_efi_get_secureboot() function which returns whether UEFI Secure Boot is enabled or not on UEFI systems. Reviewed-by: Daniel Kiper 2020-12-11 Daniel Kiper 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. Reviewed-by: Daniel Kiper 2020-12-11 Daniel Kiper 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. Reviewed-by: Daniel Kiper 2020-12-11 Daniel Kiper 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. Reviewed-by: Daniel Kiper 2020-12-11 Javier Martinez Canillas 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. Reviewed-by: Daniel Kiper 2020-12-11 Javier Martinez Canillas include/grub/i386/linux.h: Include missing header This header uses types defined in 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; Reviewed-by: Daniel Kiper 2020-12-11 Javier Martinez Canillas i386: Don't include 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' Reviewed-by: Daniel Kiper 2020-11-20 Glenn Washburn 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". Reviewed-by: Patrick Steinhardt Reviewed-by: Daniel Kiper 2020-11-20 Glenn Washburn 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. Reviewed-by: Patrick Steinhardt Reviewed-by: Daniel Kiper 2020-11-20 Glenn Washburn 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. Reviewed-by: Patrick Steinhardt Reviewed-by: Daniel Kiper 2020-11-20 Glenn Washburn 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. Reviewed-by: Patrick Steinhardt Reviewed-by: Daniel Kiper 2020-11-20 Glenn Washburn 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. Reviewed-by: Patrick Steinhardt Reviewed-by: Daniel Kiper 2020-11-20 Glenn Washburn 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. Reviewed-by: Patrick Steinhardt Reviewed-by: Daniel Kiper 2020-11-20 Glenn Washburn types: Define GRUB_CHAR_BIT based on compiler macro instead of using literal Reviewed-by: Daniel Kiper 2020-11-20 Javier Martinez Canillas include/grub/arm64/linux.h: Include missing header This header uses types defined in 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 */ | ^~~~~~~~~~~~~ Reviewed-by: Daniel Kiper 2020-11-20 Javier Martinez Canillas include/grub/arm/system.h: Include missing header The header uses the EXPORT_FUNC() macro defined in 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 Reviewed-by: Daniel Kiper 2020-11-20 Daniel Axtens docs: grub-install --pubkey has been supported for some time grub-install --pubkey is supported, so we can now document it. Reviewed-by: Daniel Kiper 2020-11-20 Daniel Axtens docs: grub-install is no longer a shell script Since commit cd46aa6cefab 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. Reviewed-by: Daniel Kiper 2020-10-30 Jacob Kroon Makefile: Remove unused GRUB_PKGLIBDIR definition Reviewed-by: Daniel Kiper 2020-10-30 Daniel Axtens 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. Reviewed-by: Daniel Kiper 2020-10-30 Cao jin kern/i386/realmode: Update comment Commit b81d609e4c did not update it. Reviewed-by: Daniel Kiper 2020-10-30 Glenn Washburn 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". Reviewed-by: Patrick Steinhardt Reviewed-by: Daniel Kiper 2020-09-18 Glenn Washburn crypto: Remove GPG_ERROR_CFLAGS from gpg_err_code_t enum This was probably added by accident when originally creating the file. Reviewed-by: Daniel Kiper 2020-09-18 Glenn Washburn 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. Reviewed-by: Daniel Kiper 2020-09-18 Glenn Washburn docs: Support for loading and concatenating multiple initrds This has been available since January of 2012 but has not been documented. Reviewed-by: Daniel Kiper 2020-09-18 Glenn Washburn lexer: char const * should be const char * Reviewed-by: Daniel Kiper cryptodisk: Use cipher name instead of object in error message Reviewed-by: Daniel Kiper 2020-09-18 Glenn Washburn 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. Acked-by: Jaegeuk Kim Tested-by: Paul Menzel Reviewed-by: Daniel Kiper 2020-09-18 Florian La Roche grub-mkconfig: If $hints is not set reduce the output into grub.cfg to just 1 line Reviewed-by: Daniel Kiper 2020-09-18 Petr Vorel 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. Reviewed-by: Daniel Kiper 2020-09-18 Patrick Steinhardt 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. Reviewed-by: Daniel Kiper 2020-09-18 Tianjia Zhang 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. Reviewed-by: Daniel Kiper 2020-09-18 Tianjia Zhang 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. Reviewed-by: Daniel Kiper 2020-09-18 Daniel Kiper efi/tpm: Fix typo in grub_efi_tpm2_protocol struct Rename get_active_pcr_blanks() to get_active_pcr_banks(). Reviewed-by: Javier Martinez Canillas 2020-09-18 Daniel Kiper i386/efi/init: Drop bogus include Reviewed-by: Javier Martinez Canillas 2020-09-18 Daniel Kiper docs: Fix devicetree command description Specifically fix the subsection and drop bogus reference to the GNU/Linux. Reported-by: Patrick Higgins Reviewed-by: Javier Martinez Canillas 2020-09-18 Martin Whitaker 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. Reviewed-by: Javier Martinez Canillas 2020-09-11 Javier Martinez Canillas tftp: Roll-over block counter to prevent data packets timeouts Commit 781b3e5efc3 (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: 781b3e5efc3 (tftp: Do not use priority queue) Suggested-by: Peter Jones Reviewed-by: Daniel Kiper 2020-09-11 Florian La Roche templates: Remove unnecessary trailing semicolon Reviewed-by: Daniel Kiper 2020-09-11 Glenn Washburn cryptodisk: Fix incorrect calculation of start sector Here dev is a grub_cryptodisk_t and dev->offset is offset in sectors of size native to the cryptodisk device. The sector is correctly transformed into native grub sector size, but then added to dev->offset which is not transformed. It would be nice if the type system would help us with this. Reviewed-by: Patrick Steinhardt Reviewed-by: Daniel Kiper 2020-09-11 Glenn Washburn cryptodisk: Unregister cryptomount command when removing module Reviewed-by: Patrick Steinhardt Reviewed-by: Daniel Kiper 2020-09-11 Patrick Steinhardt luks2: Improve error reporting when decrypting/verifying key While we already set up error messages in both luks2_verify_key() and luks2_decrypt_key(), we do not ever print them. This makes it really hard to discover why a given key actually failed to decrypt a disk. Improve this by including the error message in the user-visible output. Reviewed-by: Daniel Kiper 2020-09-11 Patrick Steinhardt luks: Fix out-of-bounds copy of UUID When configuring a LUKS disk, we copy over the UUID from the LUKS header into the new grub_cryptodisk_t structure via grub_memcpy(). As size we mistakenly use the size of the grub_cryptodisk_t UUID field, which is guaranteed to be strictly bigger than the LUKS UUID field we're copying. As a result, the copy always goes out-of-bounds and copies some garbage from other surrounding fields. During runtime, this isn't noticed due to the fact that we always NUL-terminate the UUID and thus never hit the trailing garbage. Fix the issue by using the size of the local stripped UUID field. Reviewed-by: Daniel Kiper 2020-09-11 Patrick Steinhardt json: Remove invalid typedef redefinition The C standard does not allow for typedef redefinitions, even if they map to the same underlying type. In order to avoid including the jsmn.h in json.h and thus exposing jsmn's internals, we have exactly such a forward-declaring typedef in json.h. If enforcing the GNU99 C standard, clang may generate a warning about this non-standard construct. Fix the issue by using a simple "struct jsmntok" forward declaration instead of using a typedef. Tested-by: Chuck Tuffli Reviewed-by: Daniel Kiper 2020-09-11 Cao jin i386/relocator_common: Drop empty #ifdef Reviewed-by: Daniel Kiper 2020-09-11 Ave Milia video/bochs: Fix typo Reviewed-by: Daniel Kiper 2020-07-29 Colin Watson linux: Fix integer overflows in initrd size handling These could be triggered by a crafted filesystem with very large files. Fixes: CVE-2020-15707 Reviewed-by: Jan Setje-Eilers Reviewed-by: Daniel Kiper 2020-07-29 Peter Jones loader/linux: Avoid overflow on initrd size calculation Reviewed-by: Daniel Kiper 2020-07-29 Alexey Makhalov efi: Fix use-after-free in halt/reboot path commit 92bfc33db984 ("efi: Free malloc regions on exit") introduced memory freeing in grub_efi_fini(), which is used not only by exit path but by halt/reboot one as well. As result of memory freeing, code and data regions used by modules, such as halt, reboot, acpi (used by halt) also got freed. After return to module code, CPU executes, filled by UEFI firmware (tested with edk2), 0xAFAFAFAF pattern as a code. Which leads to #UD exception later. grub> halt !!!! X64 Exception Type - 06(#UD - Invalid Opcode) CPU Apic ID - 00000000 !!!! RIP - 0000000003F4EC28, CS - 0000000000000038, RFLAGS - 0000000000200246 RAX - 0000000000000000, RCX - 00000000061DA188, RDX - 0A74C0854DC35D41 RBX - 0000000003E10E08, RSP - 0000000007F0F860, RBP - 0000000000000000 RSI - 00000000064DB768, RDI - 000000000832C5C3 R8 - 0000000000000002, R9 - 0000000000000000, R10 - 00000000061E2E52 R11 - 0000000000000020, R12 - 0000000003EE5C1F, R13 - 00000000061E0FF4 R14 - 0000000003E10D80, R15 - 00000000061E2F60 DS - 0000000000000030, ES - 0000000000000030, FS - 0000000000000030 GS - 0000000000000030, SS - 0000000000000030 CR0 - 0000000080010033, CR2 - 0000000000000000, CR3 - 0000000007C01000 CR4 - 0000000000000668, CR8 - 0000000000000000 DR0 - 0000000000000000, DR1 - 0000000000000000, DR2 - 0000000000000000 DR3 - 0000000000000000, DR6 - 00000000FFFF0FF0, DR7 - 0000000000000400 GDTR - 00000000079EEA98 0000000000000047, LDTR - 0000000000000000 IDTR - 0000000007598018 0000000000000FFF, TR - 0000000000000000 FXSAVE_STATE - 0000000007F0F4C0 Proposal here is to continue to free allocated memory for exit boot services path but keep it for halt/reboot path as it won't be much security concern here. Introduced GRUB_LOADER_FLAG_EFI_KEEP_ALLOCATED_MEMORY loader flag to be used by efi halt/reboot path. Reviewed-by: Darren Kenny Reviewed-by: Daniel Kiper 2020-07-29 Daniel Kiper efi/chainloader: Propagate errors from copy_file_path() Without any error propagated to the caller, make_file_path() would then try to advance the invalid device path node with GRUB_EFI_NEXT_DEVICE_PATH(), which would fail, returning a NULL pointer that would subsequently be dereferenced. Hence, propagate errors from copy_file_path(). Reviewed-by: Daniel Kiper 2020-07-29 Peter Jones efi: Fix some malformed device path arithmetic errors Several places we take the length of a device path and subtract 4 from it, without ever checking that it's >= 4. There are also cases where this kind of malformation will result in unpredictable iteration, including treating the length from one dp node as the type in the next node. These are all errors, no matter where the data comes from. This patch adds a checking macro, GRUB_EFI_DEVICE_PATH_VALID(), which can be used in several places, and makes GRUB_EFI_NEXT_DEVICE_PATH() return NULL and GRUB_EFI_END_ENTIRE_DEVICE_PATH() evaluate as true when the length is too small. Additionally, it makes several places in the code check for and return errors in these cases. Reviewed-by: Daniel Kiper 2020-07-29 Peter Jones emu: Make grub_free(NULL) safe The grub_free() implementation in grub-core/kern/mm.c safely handles NULL pointers, and code at many places depends on this. We don't know that the same is true on all host OSes, so we need to handle the same behavior in grub-emu's implementation. Reviewed-by: Darren Kenny Reviewed-by: Daniel Kiper 2020-07-29 Peter Jones lvm: Fix two more potential data-dependent alloc overflows It appears to be possible to make a (possibly invalid) lvm PV with a metadata size field that overflows our type when adding it to the address we've allocated. Even if it doesn't, it may be possible to do so with the math using the outcome of that as an operand. Check them both. Reviewed-by: Daniel Kiper 2020-07-29 Peter Jones hfsplus: Fix two more overflows Both node->size and node->namelen come from the supplied filesystem, which may be user-supplied. We can't trust them for the math unless we know they don't overflow. Making sure they go through grub_add() or grub_calloc() first will give us that. Reviewed-by: Darren Kenny Reviewed-by: Daniel Kiper 2020-07-29 Alexey Makhalov relocator: Fix grub_relocator_alloc_chunk_align() top memory allocation Current implementation of grub_relocator_alloc_chunk_align() does not allow allocation of the top byte. Assuming input args are: max_addr = 0xfffff000; size = 0x1000; And this is valid. But following overflow protection will unnecessarily move max_addr one byte down (to 0xffffefff): if (max_addr > ~size) max_addr = ~size; ~size + 1 will fix the situation. In addition, check size for non zero to do not zero max_addr. Reviewed-by: Daniel Kiper 2020-07-29 Chris Coulson script: Avoid a use-after-free when redefining a function during execution Defining a new function with the same name as a previously defined function causes the grub_script and associated resources for the previous function to be freed. If the previous function is currently executing when a function with the same name is defined, this results in use-after-frees when processing subsequent commands in the original function. Instead, reject a new function definition if it has the same name as a previously defined function, and that function is currently being executed. Although a behavioural change, this should be backwards compatible with existing configurations because they can't be dependent on the current behaviour without being broken. Fixes: CVE-2020-15706 Reviewed-by: Daniel Kiper 2020-07-29 Chris Coulson script: Remove unused fields from grub_script_function struct Reviewed-by: Daniel Kiper 2020-07-29 Alexey Makhalov relocator: Protect grub_relocator_alloc_chunk_align() max_addr against integer underflow This commit introduces integer underflow mitigation in max_addr calculation in grub_relocator_alloc_chunk_align() invocation. It consists of 2 fixes: 1. Introduced grub_relocator_alloc_chunk_align_safe() wrapper function to perform sanity check for min/max and size values, and to make safe invocation of grub_relocator_alloc_chunk_align() with validated max_addr value. Replace all invocations such as grub_relocator_alloc_chunk_align(..., min_addr, max_addr - size, size, ...) by grub_relocator_alloc_chunk_align_safe(..., min_addr, max_addr, size, ...). 2. Introduced UP_TO_TOP32(s) macro for the cases where max_addr is 32-bit top address (0xffffffff - size + 1) or similar. Reviewed-by: Daniel Kiper 2020-07-29 Alexey Makhalov relocator: Protect grub_relocator_alloc_chunk_addr() input args against integer underflow/overflow Use arithmetic macros from safemath.h to accomplish it. In this commit, I didn't want to be too paranoid to check every possible math equation for overflow/underflow. Only obvious places (with non zero chance of overflow/underflow) were refactored. Reviewed-by: Daniel Kiper 2020-07-29 Alexey Makhalov tftp: Do not use priority queue There is not need to reassemble the order of blocks. Per RFC 1350, server must wait for the ACK, before sending next block. Data packets can be served immediately without putting them to priority queue. Logic to handle incoming packet is this: - if packet block id equal to expected block id, then process the packet, - if packet block id is less than expected - this is retransmit of old packet, then ACK it and drop the packet, - if packet block id is more than expected - that shouldn't happen, just drop the packet. It makes the tftp receive path code simpler, smaller and faster. As a benefit, this change fixes CID# 73624 and CID# 96690, caused by following while loop: while (cmp_block (grub_be_to_cpu16 (tftph->u.data.block), data->block + 1) == 0) where tftph pointer is not moving from one iteration to another, causing to serve same packet again. Luckily, double serving didn't happen due to data->block++ during the first iteration. Fixes: CID 73624, CID 96690 Reviewed-by: Daniel Kiper 2020-07-29 Konrad Rzeszutek Wilk multiboot2: Fix memory leak if grub_create_loader_cmdline() fails Fixes: CID 292468 Reviewed-by: Daniel Kiper 2020-07-29 Konrad Rzeszutek Wilk udf: Fix memory leak Fixes: CID 73796 Reviewed-by: Daniel Kiper Reviewed-by: Jan Setje-Eilers 2020-07-29 Konrad Rzeszutek Wilk term: Fix overflow on user inputs This requires a very weird input from the serial interface but can cause an overflow in input_buf (keys) overwriting the next variable (npending) with the user choice: (pahole output) struct grub_terminfo_input_state { int input_buf[6]; /* 0 24 */ int npending; /* 24 4 */ <- CORRUPT ...snip... The magic string requires causing this is "ESC,O,],0,1,2,q" and we overflow npending with "q" (aka increase npending to 161). The simplest fix is to just to disallow overwrites input_buf, which exactly what this patch does. Fixes: CID 292449 Reviewed-by: Daniel Kiper 2020-07-29 Konrad Rzeszutek Wilk lzma: Make sure we don't dereference past array The two dimensional array p->posSlotEncoder[4][64] is being dereferenced using the GetLenToPosState() macro which checks if len is less than 5, and if so subtracts 2 from it. If len = 0, that is 0 - 2 = 4294967294. Obviously we don't want to dereference that far out so we check if the position found is greater or equal kNumLenToPosStates (4) and bail out. N.B.: Upstream LZMA 18.05 and later has this function completely rewritten without any history. Fixes: CID 51526 Reviewed-by: Daniel Kiper 2020-07-29 Chris Coulson json: Avoid a double-free when parsing fails. When grub_json_parse() succeeds, it returns the root object which contains a pointer to the provided JSON string. Callers are responsible for ensuring that this string outlives the root object and for freeing its memory when it's no longer needed. If grub_json_parse() fails to parse the provided JSON string, it frees the string before returning an error. This results in a double free in luks2_recover_key(), which also frees the same string after grub_json_parse() returns an error. This changes grub_json_parse() to never free the JSON string passed to it, and updates the documentation for it to make it clear that callers are responsible for ensuring that the string outlives the root JSON object. Fixes: CID 292465 Reviewed-by: Daniel Kiper 2020-07-29 Alexey Makhalov xnu: Fix double free in grub_xnu_devprop_add_property() grub_xnu_devprop_add_property() should not free utf8 and utf16 as it get allocated and freed in the caller. Minor improvement: do prop fields initialization after memory allocations. Fixes: CID 292442, CID 292457, CID 292460, CID 292466 Reviewed-by: Daniel Kiper 2020-07-29 Alexey Makhalov gfxmenu: Fix double free in load_image() self->bitmap should be zeroed after free. Otherwise, there is a chance to double free (USE_AFTER_FREE) it later in rescale_image(). Fixes: CID 292472 Reviewed-by: Daniel Kiper 2020-07-29 Daniel Kiper font: Do not load more than one NAME section The GRUB font file can have one NAME section only. Though if somebody crafts a broken font file with many NAME sections and loads it then the GRUB leaks memory. So, prevent against that by loading first NAME section and failing in controlled way on following one. Reported-by: Chris Coulson Reviewed-by: Jan Setje-Eilers 2020-07-29 Peter Jones iso9660: Don't leak memory on realloc() failures Reviewed-by: Daniel Kiper 2020-07-29 Peter Jones malloc: Use overflow checking primitives where we do complex allocations This attempts to fix the places where we do the following where arithmetic_expr may include unvalidated data: X = grub_malloc(arithmetic_expr); It accomplishes this by doing the arithmetic ahead of time using grub_add(), grub_sub(), grub_mul() and testing for overflow before proceeding. Among other issues, this fixes: - allocation of integer overflow in grub_video_bitmap_create() reported by Chris Coulson, - allocation of integer overflow in grub_png_decode_image_header() reported by Chris Coulson, - allocation of integer overflow in grub_squash_read_symlink() reported by Chris Coulson, - allocation of integer overflow in grub_ext2_read_symlink() reported by Chris Coulson, - allocation of integer overflow in read_section_as_string() reported by Chris Coulson. Fixes: CVE-2020-14309, CVE-2020-14310, CVE-2020-14311 Reviewed-by: Daniel Kiper 2020-07-29 Peter Jones calloc: Use calloc() at most places This modifies most of the places we do some form of: X = malloc(Y * Z); to use calloc(Y, Z) instead. Among other issues, this fixes: - allocation of integer overflow in grub_png_decode_image_header() reported by Chris Coulson, - allocation of integer overflow in luks_recover_key() reported by Chris Coulson, - allocation of integer overflow in grub_lvm_detect() reported by Chris Coulson. Fixes: CVE-2020-14308 Reviewed-by: Daniel Kiper 2020-07-29 Peter Jones calloc: Make sure we always have an overflow-checking calloc() available This tries to make sure that everywhere in this source tree, we always have an appropriate version of calloc() (i.e. grub_calloc(), xcalloc(), etc.) available, and that they all safely check for overflow and return NULL when it would occur. Reviewed-by: Daniel Kiper 2020-07-29 Peter Jones safemath: Add some arithmetic primitives that check for overflow This adds a new header, include/grub/safemath.h, that includes easy to use wrappers for __builtin_{add,sub,mul}_overflow() declared like: bool OP(a, b, res) where OP is grub_add, grub_sub or grub_mul. OP() returns true in the case where the operation would overflow and res is not modified. Otherwise, false is returned and the operation is executed. These arithmetic primitives require newer compiler versions. So, bump these requirements in the INSTALL file too. Reviewed-by: Daniel Kiper 2020-07-29 Peter Jones yylex: Make lexer fatal errors actually be fatal When presented with a command that can't be tokenized to anything smaller than YYLMAX characters, the parser calls YY_FATAL_ERROR(errmsg), expecting that will stop further processing, as such: #define YY_DO_BEFORE_ACTION \ yyg->yytext_ptr = yy_bp; \ yyleng = (int) (yy_cp - yy_bp); \ yyg->yy_hold_char = *yy_cp; \ *yy_cp = '\0'; \ if ( yyleng >= YYLMAX ) \ YY_FATAL_ERROR( "token too large, exceeds YYLMAX" ); \ yy_flex_strncpy( yytext, yyg->yytext_ptr, yyleng + 1 , yyscanner); \ yyg->yy_c_buf_p = yy_cp; The code flex generates expects that YY_FATAL_ERROR() will either return for it or do some form of longjmp(), or handle the error in some way at least, and so the strncpy() call isn't in an "else" clause, and thus if YY_FATAL_ERROR() is *not* actually fatal, it does the call with the questionable limit, and predictable results ensue. Unfortunately, our implementation of YY_FATAL_ERROR() is: #define YY_FATAL_ERROR(msg) \ do { \ grub_printf (_("fatal error: %s\n"), _(msg)); \ } while (0) The same pattern exists in yyless(), and similar problems exist in users of YY_INPUT(), several places in the main parsing loop, yy_get_next_buffer(), yy_load_buffer_state(), yyensure_buffer_stack, yy_scan_buffer(), etc. All of these callers expect YY_FATAL_ERROR() to actually be fatal, and the things they do if it returns after calling it are wildly unsafe. Fixes: CVE-2020-10713 Reviewed-by: Daniel Kiper 2020-05-25 Marc Zyngier arm: Fix 32-bit ARM handling of the CTR register When booting on an ARMv8 core that implements either CTR.IDC or CTR.DIC (indicating that some of the cache maintenance operations can be removed when dealing with I/D-cache coherency, GRUB dies with a "Unsupported cache type 0x........" message. This is pretty likely to happen when running in a virtual machine hosted on an arm64 machine (I've triggered it on a system built around a bunch of Cortex-A55 cores, which implements CTR.IDC). It turns out that the way GRUB deals with the CTR register is a bit harsh for anything from ARMv7 onwards. The layout of the register is backward compatible, meaning that nothing that gets added is allowed to break earlier behaviour. In this case, ignoring IDC is completely fine, and only results in unnecessary cache maintenance. We can thus avoid being paranoid, and align the 32bit behaviour with its 64bit equivalent. This patch has the added benefit that it gets rid of a (gnu-specific) case range too. Reviewed-by: Leif Lindholm Reviewed-by: Daniel Kiper 2020-05-25 Ian Jackson templates/20_linux_xen: Support Xen Security Modules (XSM/FLASK) XSM is enabled by adding "flask=enforcing" as a Xen command line argument, and providing the policy file as a grub module. We make entries for both with and without XSM. If XSM is not compiled into Xen, then there are no policy files, so no change to the boot options. Reviewed-by: Daniel Kiper 2020-05-25 Ian Jackson templates/20_linux_xen: Ignore xenpolicy and config files too file_is_not_sym() currently only checks for xen-syms. Extend it to disregard xenpolicy (XSM policy files) and files ending .config (which are built by the Xen upstream build system in some configurations and can therefore end up in /boot). Rename the function accordingly, to file_is_not_xen_garbage(). Reviewed-by: Daniel Kiper 2020-05-25 Javier Martinez Canillas net: Break out nested function Nested functions are not supported in C, but are permitted as an extension in the GNU C dialect. Commit cb2f15c5448 ("normal/main: Search for specific config files for netboot") added a nested function which caused the build to break when compiling with clang. Break that out into a static helper function to make the code portable again. Reported-by: Daniel Axtens Tested-by: Daniel Axtens Reviewed-by: Daniel Kiper 2020-05-25 Javier Martinez Canillas tpm: Enable module for all EFI platforms The module is only enabled for x86_64, but there's nothing specific to x86_64 in the implementation and can be enabled for all EFI platforms. Reviewed-by: Daniel Kiper 2020-05-25 Daniel Kiper INSTALL/configure: Update install doc and configure comment ..to reflect the GRUB build reality in them. Additionally, fix text formatting a bit. Reviewed-by: Leif Lindholm 2020-05-25 Daniel Kiper configure: Set gnu99 C language standard by default Commit d5a32255d (misc: Make grub_strtol() "end" pointers have safer const qualifiers) introduced "restrict" keyword into some functions definitions. This keyword was introduced in C99 standard. However, some compilers by default may use C89 or something different. This behavior leads to the breakage during builds when c89 or gnu89 is in force. So, let's set gnu99 C language standard for all compilers by default. This way a bit random build issue will be fixed and the GRUB source will be build consistently regardless of type and version of the compiler. It was decided to use gnu99 C language standard because it fixes the issue mentioned above and also provides some useful extensions which are used here and there in the GRUB source. Potentially we can use gnu11 too. However, this may reduce pool of older compilers which can be used to build the GRUB. So, let's live with gnu99 until we discover that we strongly require a feature from newer C standard. The user is still able to override C language standard using relevant *_CFLAGS variables. Reviewed-by: Leif Lindholm 2020-05-15 Tianjia Zhang tpm: Rename function grub_tpm_log_event() to grub_tpm_measure() grub_tpm_log_event() and grub_tpm_measure() are two functions that have the same effect. So, keep grub_tpm_log_event() and rename it to grub_tpm_measure(). This way we get also a more clear semantics. Reviewed-by: Daniel Kiper 2020-05-15 Daniel Kiper autogen: Replace -iname with -ipath in find command ..because -iname cannot be used to match paths. Reviewed-by: Javier Martinez Canillas Reviewed-by: Leif Lindholm Reviewed-by: Daniel Axtens 2020-05-15 Daniel Kiper INSTALL: Update configure example ..to make it more relevant. Reviewed-by: Leif Lindholm 2020-05-15 Daniel Kiper configure: Drop unneeded TARGET_CFLAGS expansion Reviewed-by: Javier Martinez Canillas Reviewed-by: Leif Lindholm 2020-05-15 Jacob Kroon docs/grub: Support for probing partition UUID on MSDOS disks Support was implemented in commit c7cb11b21 (probe: Support probing for msdos PARTUUID). Reviewed-by: Daniel Kiper 2020-05-15 Tianjia Zhang verifiers: Add verify string debug message Like grub_verifiers_open(), the grub_verify_string() should also display this debug message, which is very helpful for debugging. Reviewed-by: Daniel Kiper 2020-05-15 Javier Martinez Canillas envblk: Fix buffer overrun when attempting to shrink a variable value If an existing variable is set with a value whose length is smaller than the current value, a memory corruption can happen due copying padding '#' characters outside of the environment block buffer. This is caused by a wrong calculation of the previous free space position after moving backward the characters that followed the old variable value. That position is calculated to fill the remaining of the buffer with the padding '#' characters. But since isn't calculated correctly, it can lead to copies outside of the buffer. The issue can be reproduced by creating a variable with a large value and then try to set a new value that is much smaller: $ grub2-editenv --version grub2-editenv (GRUB) 2.04 $ grub2-editenv env create $ grub2-editenv env set a="$(for i in {1..500}; do var="b$var"; done; echo $var)" $ wc -c env 1024 grubenv $ grub2-editenv env set a="$(for i in {1..50}; do var="b$var"; done; echo $var)" malloc(): corrupted top size Aborted (core dumped) $ wc -c env 0 grubenv Reported-by: Renaud Métrich Reviewed-by: Daniel Kiper 2020-05-15 Hans Ulrich Niedermann docs: Remove docs for non-existing uppermem command Remove all documentation of and mentions of the uppermem command from the docs/grub.texi file. The uppermem command is not implemented in the GRUB source at all and appears to never have been implemented despite former plans to add an uppermem command. To reduce user confusion, this even removes the paragraph describing how GRUB's uppermem command was supposed to complement the Linux kernel's mem= parameter. Reviewed-by: Daniel Kiper 2020-05-15 Hans Ulrich Niedermann docs: Remove docs for non-existing pxe_unload command Remove the documentation of the pxe_unload command from the docs/grub.texi file. The pxe_unload command is not implemented in the grub source at this time at all. It appears to have been removed in commit 671a78acb (cleanup pxe and efi network release). Reviewed-by: Daniel Kiper 2020-05-15 Hans Ulrich Niedermann gitignore: Add a few forgotten file patterns Add a few patterns to .gitignore to cover files which are generated by building grub ("make", "make check", "make dist") but which have been forgotten to add to .gitignore in the past. Reviewed-by: Daniel Kiper 2020-05-15 Hans Ulrich Niedermann gitignore: Add leading slashes where appropriate Going through the list of gitignore patterns without a leading slash, this adds a leading slash where it appears to have been forgotten. Some gitignore patterns like ".deps/" or "Makefile" clearly should match everywhere, so those definitively need no leading slash. For some patterns like "ascii.bitmaps", it is unclear where in the source tree they should match. Those patterns are kept as they are, matching the patterns in the whole tree of subdirectories. Reviewed-by: Daniel Kiper 2020-05-15 Hans Ulrich Niedermann gitignore: Add trailing slashes for directories Add trailing slashes for all patterns matching directories. Note that we do *not* add trailing slashes for *symlinks* to directories. Reviewed-by: Daniel Kiper 2020-05-15 Hans Ulrich Niedermann gitignore: Sort both pattern groups alphabetically Alphabetically sort the two groups of gitignore patterns: * The group of patterns without slashes, matching anywhere in the directory subtree. * The group of patterns with slashes, matching relative to the .gitignore file's directory Reviewed-by: Daniel Kiper 2020-05-15 Hans Ulrich Niedermann gitignore: Group patterns with and without slash Group the .gitignore patterns into two groups: * Pattern not including a slash, i.e. matching files anywhere in the .gitignore file's directory and all of its subdirectories. * Patterns including a slash, i.e. matching only relative to the .gitignore file's directory. Reviewed-by: Daniel Kiper 2020-05-15 Hans Ulrich Niedermann gitignore: Consistent leading slash is easier to read As all gitignore patterns containing a left or middle slash match only relative to the .gitignore file's directory, we write them all in the same manner with a leading slash. This makes the file significantly easier to read. Reviewed-by: Daniel Kiper 2020-05-15 Daniel Kiper mips/cache: Add missing nop's in delay slots Lack of them causes random instructions to be executed before the jump really happens. Reviewed-by: Daniel Kiper 2020-04-21 Patrick Steinhardt luks2: Propagate error when reading area key fails When decrypting a given keyslot, all error cases except for one set up an error and return the error code. The only exception is when we try to read the area key: instead of setting up an error message, we directly print it via grub_dprintf(). Convert the outlier to use grub_error() to allow more uniform handling of errors. Reviewed-by: Daniel Kiper 2020-04-21 Patrick Steinhardt json: Get rid of casts for "jsmntok_t" With the upstream change having landed that adds a name to the previously anonymous "jsmntok" typedef, we can now add a forward declaration for that struct in our code. As a result, we no longer have to store the "tokens" member of "struct grub_json" as a void pointer but can instead use the forward declaration, allowing us to get rid of casts of that field. Reviewed-by: Daniel Kiper 2020-04-21 Patrick Steinhardt json: Update jsmn library to upstream commit 053d3cd Update our embedded version of the jsmn library to upstream commit 053d3cd (Merge pull request #175 from pks-t/pks/struct-type, 2020-04-02). Reviewed-by: Daniel Kiper 2020-04-21 Steve Langasek templates: Output a menu entry for firmware setup on UEFI FastBoot systems The fwsetup command allows to reboot into the EFI firmware setup menu, add a template to include a menu entry on EFI systems that makes use of that command to reboot into the EFI firmware settings. This is useful for users since the hotkey to enter into the EFI setup menu may not be the same on all systems so users can use the menu entry without needing to figure out what key needs to be pressed. Also, if fastboot is enabled in the BIOS then often it is not possible to enter the firmware setup menu. So the entry is again useful for this case. Reviewed-by: Daniel Kiper 2020-04-21 Hans de Goede kern/term: Accept ESC, F4 and holding SHIFT as user interrupt keys On some devices the ESC key is the hotkey to enter the BIOS/EFI setup screen, making it really hard to time pressing it right. Besides that ESC is also pretty hard to discover for a user who does not know it will unhide the menu. This commit makes F4, which was chosen because is not used as a hotkey to enter the BIOS setup by any vendor, also interrupt sleeps / stop the menu countdown. This solves the ESC gets into the BIOS setup and also somewhat solves the discoverability issue, but leaves the timing issue unresolved. This commit fixes the timing issue by also adding support for keeping SHIFT pressed during boot to stop the menu countdown. This matches what Ubuntu is doing, which should also help with discoverability. Reviewed-by: Daniel Kiper 2020-04-21 Hans de Goede efi/console: Do not set text-mode until we actually need it If we're running with a hidden menu we may never need text mode, so do not change the video-mode to text until we actually need it. This allows to boot a machine without unnecessary graphical transitions and provide a seamless boot experience to users. Reviewed-by: Daniel Kiper 2020-04-21 Hans de Goede efi/console: Implement getkeystatus() support Implement getkeystatus() support in the EFI console driver. This is needed because the logic to determine if a key was pressed to make the menu countdown stop will be changed by a later patch to also take into account the SHIFT key being held down. For this reason the EFI console driver has to support getkeystatus() to allow detecting that event. Note that if a non-modifier key gets pressed and repeated calls to getkeystatus() are made then it will return the modifier status at the time of the non-modifier key, until that key-press gets consumed by a getkey() call. This is a side-effect of how the EFI simple-text-input protocol works and cannot be avoided. Reviewed-by: Daniel Kiper 2020-04-21 Hans de Goede efi/console: Add grub_console_read_key_stroke() helper function This is a preparatory patch for adding getkeystatus() support to the EFI console driver. We can get modifier status through the simple_text_input read_key_stroke() method, but if a non-modifier key is (also) pressed the read_key_stroke() call will consume that key from the firmware's queue. The new grub_console_read_key_stroke() helper buffers upto 1 key-stroke. If it has a non-modifier key buffered, it will return that one, if its buffer is empty, it will fills its buffer by getting a new key-stroke. If called with consume=1 it will empty its buffer after copying the key-data to the callers buffer, this is how getkey() will use it. If called with consume=0 it will keep the last key-stroke buffered, this is how getkeystatus() will call it. This means that if a non-modifier key gets pressed, repeated getkeystatus() calls will return the modifiers of that key-press until it is consumed by a getkey() call. Reviewed-by: Daniel Kiper 2020-04-21 Hans de Goede kern/term: Make grub_getkeystatus() helper function available everywhere Move grub_getkeystatushelper() function from grub-core/commands/keystatus.c to grub-core/kern/term.c and export it so that it can be used outside of the keystatus command code too. There's no logic change in this patch. The function definition is moved so it can be called from grub-core/kern/term.c in a subsequent patch. It will be used to determine if a SHIFT key has was held down and use that also to interrupt the countdown, without the need to press a key at the right time. Reviewed-by: Daniel Kiper 2020-04-21 Javier Martinez Canillas efi/console: Move grub_console_set{colorstate,cursor} higher in the file This is just a preparatory patch to move the functions higher in the file, since these will be called by the grub_prepare_for_text_output() function that will be introduced in a later patch. The logic is unchanged by this patch. Functions definitions are just moved to avoid a forward declaration in a later patch, keeping the code clean. Reviewed-by: Daniel Kiper 2020-04-21 Paul Menzel docs/grub: Fix typo in *preferred* Reviewed-by: Daniel Kiper 2020-04-21 Daniel Axtens powerpc/mkimage: Fix CHRP note descsz Currently, an image generated with 'grub-mkimage -n' causes an error when read with 'readelf -a': Displaying notes found at file offset 0x000106f0 with length 0x0000002c: Owner Data size Description readelf: Warning: note with invalid namesz and/or descsz found at offset 0x0 readelf: Warning: type: 0x1275, namesize: 0x00000008, descsize: 0x0000002c, alignment: 4 This is because the descsz of the CHRP note is set to sizeof (struct grub_ieee1275_note) which is the size of the entire note, including name and elf header. The desczs should contain only the contents, not the name and header sizes. Set the descsz instead to 'sizeof (struct grub_ieee1275_note_desc)' Resultant readelf output: Displaying notes found at file offset 0x00010710 with length 0x0000002c: Owner Data size Description PowerPC 0x00000018 Unknown note type: (0x00001275) description data: ff ff ff ff 00 c0 00 00 ff ff ff ff ff ff ff ff ff ff ff ff 00 00 40 00 So far as I can tell this issue has existed for as long as the note generation code has existed, but I guess nothing really checks descsz. Reviewed-by: Daniel Kiper 2020-03-31 Flavio Suligoi efi: Add missed space in GRUB_EFI_GLOBAL_VARIABLE_GUID Reviewed-by: Daniel Kiper 2020-03-31 Michael Chang zfs: Fix gcc10 error -Werror=zero-length-bounds We bumped into the build error while testing gcc-10 pre-release. In file included from ../../include/grub/file.h:22, from ../../grub-core/fs/zfs/zfs.c:34: ../../grub-core/fs/zfs/zfs.c: In function 'zap_leaf_lookup': ../../grub-core/fs/zfs/zfs.c:2263:44: error: array subscript '' is outside the bounds of an interior zero-length array 'grub_uint16_t[0]' {aka 'short unsigned int[0]'} [-Werror=zero-length-bounds] 2263 | for (chunk = grub_zfs_to_cpu16 (l->l_hash[LEAF_HASH (blksft, h, l)], endian); ../../include/grub/types.h:241:48: note: in definition of macro 'grub_le_to_cpu16' 241 | # define grub_le_to_cpu16(x) ((grub_uint16_t) (x)) | ^ ../../grub-core/fs/zfs/zfs.c:2263:16: note: in expansion of macro 'grub_zfs_to_cpu16' 2263 | for (chunk = grub_zfs_to_cpu16 (l->l_hash[LEAF_HASH (blksft, h, l)], endian); | ^~~~~~~~~~~~~~~~~ In file included from ../../grub-core/fs/zfs/zfs.c:48: ../../include/grub/zfs/zap_leaf.h:72:16: note: while referencing 'l_hash' 72 | grub_uint16_t l_hash[0]; | ^~~~~~ Here I'd like to quote from the gcc document [1] which seems best to explain what is going on here. "Although the size of a zero-length array is zero, an array member of this kind may increase the size of the enclosing type as a result of tail padding. The offset of a zero-length array member from the beginning of the enclosing structure is the same as the offset of an array with one or more elements of the same type. The alignment of a zero-length array is the same as the alignment of its elements. Declaring zero-length arrays in other contexts, including as interior members of structure objects or as non-member objects, is discouraged. Accessing elements of zero-length arrays declared in such contexts is undefined and may be diagnosed." The l_hash[0] is apparnetly an interior member to the enclosed structure while l_entries[0] is the trailing member. And the offending code tries to access members in l_hash[0] array that triggers the diagnose. Given that the l_entries[0] is used to get proper alignment to access leaf chunks, we can accomplish the same thing through the ALIGN_UP macro thus eliminating l_entries[0] from the structure. In this way we can pacify the warning as l_hash[0] now becomes the last member to the enclosed structure. [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html Reviewed-by: Daniel Kiper 2020-03-31 Michael Chang mdraid1x_linux: Fix gcc10 error -Werror=array-bounds We bumped into the build error while testing gcc-10 pre-release. ../../grub-core/disk/mdraid1x_linux.c: In function 'grub_mdraid_detect': ../../grub-core/disk/mdraid1x_linux.c:181:15: error: array subscript is outside array bounds of 'grub_uint16_t[0]' {aka 'short unsigned int[0]'} [-Werror=array-bounds] 181 | (char *) &sb.dev_roles[grub_le_to_cpu32 (sb.dev_number)] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../../grub-core/disk/mdraid1x_linux.c:98:17: note: while referencing 'dev_roles' 98 | grub_uint16_t dev_roles[0]; /* Role in array, or 0xffff for a spare, or 0xfffe for faulty. */ | ^~~~~~~~~ ../../grub-core/disk/mdraid1x_linux.c:127:33: note: defined here 'sb' 127 | struct grub_raid_super_1x sb; | ^~ cc1: all warnings being treated as errors Apparently gcc issues the warning when trying to access sb.dev_roles array's member, since it is a zero length array as the last element of struct grub_raid_super_1x that is allocated sparsely without extra chunks for the trailing bits, so the warning looks legitimate in this regard. As the whole thing here is doing offset computation, it is undue to use syntax that would imply array member access then take address from it later. Instead we could accomplish the same thing through basic array pointer arithmetic to pacify the warning. Reviewed-by: Daniel Kiper 2020-03-31 Simon Hardy build: Fix GRUB i386-pc build with Ubuntu gcc With recent versions of gcc on Ubuntu a very large lzma_decompress.img file is output. (e.g. 134479600 bytes instead of 2864.) This causes grub-mkimage to fail with: "error: Decompressor is too big." This seems to be caused by a section .note.gnu.property that is placed at an offset such that objcopy needs to pad the img file with zeros. This issue is present on: Ubuntu 19.10 with gcc (Ubuntu 8.3.0-26ubuntu1~19.10) 8.3.0 Ubuntu 19.10 with gcc (Ubuntu 9.2.1-9ubuntu2) 9.2.1 20191008 This issue is not present on: Ubuntu 19.10 with gcc (Ubuntu 7.5.0-3ubuntu1~19.10) 7.5.0 RHEL 8.0 with gcc 8.3.1 20190507 (Red Hat 8.3.1-4) The issue can be fixed by removing the section using objcopy as shown in this patch. Reviewed-by: Daniel Kiper 2020-03-31 Tianjia Zhang efi/tpm: Fix memory leak in grub_tpm1/2_log_event() The memory requested for the event is not released here, causing memory leaks. This patch fixes this problem. Reviewed-by: Javier Martinez Canillas Reviewed-by: Daniel Kiper 2020-03-31 Michael Chang docs: Document notes on LVM cache booting Add notes on LVM cache booting to the GRUB manual to help user understanding the outstanding issue and status. Reviewed-by: Daniel Kiper 2020-03-31 Michael Chang lvm: Add LVM cache logical volume handling The LVM cache logical volume is the logical volume consisting of the original and the cache pool logical volume. The original is usually on a larger and slower storage device while the cache pool is on a smaller and faster one. The performance of the original volume can be improved by storing the frequently used data on the cache pool to utilize the greater performance of faster device. The default cache mode "writethrough" ensures that any data written will be stored both in the cache and on the origin LV, therefore grub can be straight to read the original lv as no data loss is guarenteed. The second cache mode is "writeback", which delays writing from the cache pool back to the origin LV to have increased performance. The drawback is potential data loss if losing the associated cache device. During the boot time grub reads the LVM offline i.e. LVM volumes are not activated and mounted, hence it should be fine to read directly from original lv since all cached data should have been flushed back in the process of taking it offline. It is also not much helpful to the situation by adding fsync calls to the install code. The fsync did not force to write back dirty cache to the original device and rather it would update associated cache metadata to complete the write transaction with the cache device. IOW the writes to cached blocks still go only to the cache device. To write back dirty cache, as LVM cache did not support dirty cache flush per block range, there'no way to do it for file. On the other hand the "cleaner" policy is implemented and can be used to write back "all" dirty blocks in a cache, which effectively drain all dirty cache gradually to attain and last in the "clean" state, which can be useful for shrinking or decommissioning a cache. The result and effect is not what we are looking for here. In conclusion, as it seems no way to enforce file writes to the original device, grub may suffer from power failure as it cannot assemble the cache device and read the dirty data from it. However since the case is only applicable to writeback mode which is sensitive to data lost in nature, I'd still like to propose my (relatively simple) patch and treat reading dirty cache as improvement. Reviewed-by: Daniel Kiper 2020-03-10 Patrick Steinhardt gnulib: Fix build of base64 when compiling with memory debugging When building GRUB with memory management debugging enabled, then the build fails because of `grub_debug_malloc()` and `grub_debug_free()` being undefined in the luks2 module. The cause is that we patch "base64.h" to unconditionaly include "config-util.h", which shouldn't be included for modules at all. As a result, `MM_DEBUG` is defined when building the module, causing it to use the debug memory allocation functions. As these are not built into modules, we end up with a linker error. Fix the issue by removing the include altogether. The sole reason it was included was for the `_GL_ATTRIBUTE_CONST` macro, which we can simply define as empty in case it's not set. Reviewed-by: Daniel Kiper 2020-03-10 Patrick Steinhardt build: Fix option to explicitly disable memory debugging The memory management system supports a debug mode that can be enabled at build time by passing "--enable-mm-debug" to the configure script. Passing the option will cause us define MM_DEBUG as expected, but in fact the reverse option "--disable-mm-debug" will do the exact same thing and also set up the define. This currently causes the build of "lib/gnulib/base64.c" to fail as it tries to use `grub_debug_malloc()` and `grub_debug_free()` even though both symbols aren't defined. Seemingly, `AC_ARG_ENABLE()` will always execute the third argument if either the positive or negative option was passed. Let's thus fix the issue by moving the call to`AC_DEFINE()` into an explicit `if test $xenable_mm_debug` block, similar to how other defines work. Reviewed-by: Daniel Kiper Reviewed-by: Paul Menzel 2020-03-10 David Michael fat: Support file modification times This allows comparing file ages on EFI system partitions. Reviewed-by: Daniel Kiper 2020-03-10 David Michael exfat: Save the matching directory entry struct when searching This provides the node's attributes outside the iterator function so the file modification time can be accessed and reported. Reviewed-by: Daniel Kiper 2020-03-10 Mike Gilbert datetime: Enable the datetime module for the emu platform Fixes a build failure: grub-core/commands/date.c:49: undefined reference to `grub_get_weekday_name' grub-core/commands/ls.c:155: undefined reference to `grub_unixtime2datetime' Bug: https://bugs.gentoo.org/711512 Reviewed-by: Javier Martinez Canillas Tested-by: Javier Martinez Canillas Reviewed-by: Daniel Kiper 2020-03-10 John Paul Adrian Glaubitz build: Add soft-float handling for SuperH (sh4) While GRUB has no platform support for SuperH (sh4) yet, this change adds the target-specific handling of soft-floats such that the GRUB utilities can be built on this target. Reviewed-by: Daniel Kiper 2020-03-10 Peter Jones efi: Fix the type of grub_efi_status_t Currently, in some builds with some checkers, we see: 1. grub-core/disk/efi/efidisk.c:601: error[shiftTooManyBitsSigned]: Shifting signed 64-bit value by 63 bits is undefined behaviour This is because grub_efi_status_t is defined as grub_efi_intn_t, which is signed, and shifting into the sign bit is not defined behavior. UEFI fixed this in the spec in 2.3: 2.3 | Change the defined type of EFI_STATUS from INTN to UINTN | May 7, 2009 And the current EDK2 code has: MdePkg/Include/Base.h-// MdePkg/Include/Base.h-// Status codes common to all execution phases MdePkg/Include/Base.h-// MdePkg/Include/Base.h:typedef UINTN RETURN_STATUS; MdePkg/Include/Base.h- MdePkg/Include/Base.h-/** MdePkg/Include/Base.h- Produces a RETURN_STATUS code with the highest bit set. MdePkg/Include/Base.h- MdePkg/Include/Base.h- @param StatusCode The status code value to convert into a warning code. MdePkg/Include/Base.h- StatusCode must be in the range 0x00000000..0x7FFFFFFF. MdePkg/Include/Base.h- MdePkg/Include/Base.h- @return The value specified by StatusCode with the highest bit set. MdePkg/Include/Base.h- MdePkg/Include/Base.h-**/ MdePkg/Include/Base.h-#define ENCODE_ERROR(StatusCode) ((RETURN_STATUS)(MAX_BIT | (StatusCode))) MdePkg/Include/Base.h- MdePkg/Include/Base.h-/** MdePkg/Include/Base.h- Produces a RETURN_STATUS code with the highest bit clear. MdePkg/Include/Base.h- MdePkg/Include/Base.h- @param StatusCode The status code value to convert into a warning code. MdePkg/Include/Base.h- StatusCode must be in the range 0x00000000..0x7FFFFFFF. MdePkg/Include/Base.h- MdePkg/Include/Base.h- @return The value specified by StatusCode with the highest bit clear. MdePkg/Include/Base.h- MdePkg/Include/Base.h-**/ MdePkg/Include/Base.h-#define ENCODE_WARNING(StatusCode) ((RETURN_STATUS)(StatusCode)) MdePkg/Include/Base.h- MdePkg/Include/Base.h-/** MdePkg/Include/Base.h- Returns TRUE if a specified RETURN_STATUS code is an error code. MdePkg/Include/Base.h- MdePkg/Include/Base.h- This function returns TRUE if StatusCode has the high bit set. Otherwise, FALSE is returned. MdePkg/Include/Base.h- MdePkg/Include/Base.h- @param StatusCode The status code value to evaluate. MdePkg/Include/Base.h- MdePkg/Include/Base.h- @retval TRUE The high bit of StatusCode is set. MdePkg/Include/Base.h- @retval FALSE The high bit of StatusCode is clear. MdePkg/Include/Base.h- MdePkg/Include/Base.h-**/ MdePkg/Include/Base.h-#define RETURN_ERROR(StatusCode) (((INTN)(RETURN_STATUS)(StatusCode)) < 0) ... Uefi/UefiBaseType.h:typedef RETURN_STATUS EFI_STATUS; This patch makes grub's implementation match the Edk2 declaration with regards to the signedness of the type. Reviewed-by: Daniel Kiper 2020-03-10 Peter Jones efi/gop: Add debug output on GOP probing Add debug information to EFI GOP video driver probing function. Reviewed-by: Daniel Kiper 2020-03-10 Peter Jones efi/uga: Use video instead of fb as debug condition All other video drivers use "video" as the debug condition instead of "fb" so change this in the efi/uga driver to make it consistent with the others. Reviewed-by: Daniel Kiper 2020-03-10 Peter Jones efi: Print error messages to grub_efi_allocate_pages_real() No messages were printed in this function, add some to ease debugging. Also, the function returns a void * pointer so return NULL instead of 0 to make the code more readable. Reviewed-by: Daniel Kiper 2020-03-10 Andrei Borzenkov efi/uga: Use 64 bit for fb_base We get 64 bit from PCI BAR but then truncate by assigning to 32 bit. Make sure to check that pointer does not overflow on 32 bit platform. Closes: 50931 Reviewed-by: Daniel Kiper 2020-03-10 Alexander Graf efi/gop: Add support for BLT_ONLY adapters EFI GOP has support for multiple different bitness types of frame buffers and for a special "BLT only" type which is always defined to be RGBx. Because grub2 doesn't ever directly access the frame buffer but instead only renders graphics via the BLT interface anyway, we can easily support these adapters. The reason this has come up now is the emerging support for virtio-gpu in OVMF. That adapter does not have the notion of a memory mapped frame buffer and thus is BLT only. Reviewed-by: Daniel Kiper 2020-03-10 Peter Jones normal/completion: Fix possible NULL pointer dereference Coverity Scan reports that the grub_strrchr() function can return NULL if the character is not found. Check if that's the case for dirfile pointer. Reviewed-by: Daniel Kiper 2020-03-10 Peter Jones kern: Add grub_debug_enabled() Add a grub_debug_enabled() helper function instead of open coding it. Reviewed-by: Daniel Kiper 2020-03-10 Peter Jones Makefile: Make libgrub.pp depend on config-util.h If you build with "make -j48" a lot, sometimes you see: gcc -E -DHAVE_CONFIG_H -I. -I.. -Wall -W -DGRUB_UTIL=1 -D_FILE_OFFSET_BITS=64 -I./include -DGRUB_FILE=\"grub_script.tab.h\" -I. -I.. -I. -I.. -I../include -I./include -I../grub-core/lib/libgcrypt-grub/src/ -I../grub-core/lib/minilzo -I../grub-core/lib/xzembed -DMINILZO_HAVE_CONFIG_H -Wall -W -DGRUB_UTIL=1 -D_FILE_OFFSET_BITS=64 -I./include -DGRUB_FILE=\"grub_script.tab.h\" -I. -I.. -I. -I.. -I../include -I./include -I../grub-core/lib/libgcrypt-grub/src/ -I./grub-core/gnulib -I../grub-core/gnulib -I/builddir/build/BUILD/grub-2.02/grub-aarch64-efi-2.02 -D_FILE_OFFSET_BITS=64 \ -D'GRUB_MOD_INIT(x)=@MARKER@x@' grub_script.tab.h grub_script.yy.h ../grub-core/commands/blocklist.c ../grub-core/commands/macbless.c ../grub-core/commands/xnu_uuid.c ../grub-core/commands/testload.c ../grub-core/commands/ls.c ../grub-core/disk/dmraid_nvidia.c ../grub-core/disk/loopback.c ../grub-core/disk/lvm.c ../grub-core/disk/mdraid_linux.c ../grub-core/disk/mdraid_linux_be.c ../grub-core/disk/mdraid1x_linux.c ../grub-core/disk/raid5_recover.c ../grub-core/disk/raid6_recover.c ../grub-core/font/font.c ../grub-core/gfxmenu/font.c ../grub-core/normal/charset.c ../grub-core/video/fb/fbblit.c ../grub-core/video/fb/fbutil.c ../grub-core/video/fb/fbfill.c ../grub-core/video/fb/video_fb.c ../grub-core/video/video.c ../grub-core/video/capture.c ../grub-core/video/colors.c ../grub-core/unidata.c ../grub-core/io/bufio.c ../grub-core/fs/affs.c ../grub-core/fs/afs.c ../grub-core/fs/bfs.c ../grub-core/fs/btrfs.c ../grub-core/fs/cbfs.c ../grub-core/fs/cpio.c ../grub-core/fs/cpio_be.c ../grub-core/fs/odc.c ../grub-core/fs/newc.c ../grub-core/fs/ext2.c ../grub-core/fs/fat.c ../grub-core/fs/exfat.c ../grub-core/fs/fshelp.c ../grub-core/fs/hfs.c ../grub-core/fs/hfsplus.c ../grub-core/fs/hfspluscomp.c ../grub-core/fs/iso9660.c ../grub-core/fs/jfs.c ../grub-core/fs/minix.c ../grub-core/fs/minix2.c ../grub-core/fs/minix3.c ../grub-core/fs/minix_be.c ../grub-core/fs/minix2_be.c ../grub-core/fs/minix3_be.c ../grub-core/fs/nilfs2.c ../grub-core/fs/ntfs.c ../grub-core/fs/ntfscomp.c ../grub-core/fs/reiserfs.c ../grub-core/fs/romfs.c ../grub-core/fs/sfs.c ../grub-core/fs/squash4.c ../grub-core/fs/tar.c ../grub-core/fs/udf.c ../grub-core/fs/ufs2.c ../grub-core/fs/ufs.c ../grub-core/fs/ufs_be.c ../grub-core/fs/xfs.c ../grub-core/fs/zfs/zfscrypt.c ../grub-core/fs/zfs/zfs.c ../grub-core/fs/zfs/zfsinfo.c ../grub-core/fs/zfs/zfs_lzjb.c ../grub-core/fs/zfs/zfs_lz4.c ../grub-core/fs/zfs/zfs_sha256.c ../grub-core/fs/zfs/zfs_fletcher.c ../grub-core/lib/envblk.c ../grub-core/lib/hexdump.c ../grub-core/lib/LzFind.c ../grub-core/lib/LzmaEnc.c ../grub-core/lib/crc.c ../grub-core/lib/adler32.c ../grub-core/lib/crc64.c ../grub-core/normal/datetime.c ../grub-core/normal/misc.c ../grub-core/partmap/acorn.c ../grub-core/partmap/amiga.c ../grub-core/partmap/apple.c ../grub-core/partmap/sun.c ../grub-core/partmap/plan.c ../grub-core/partmap/dvh.c ../grub-core/partmap/sunpc.c ../grub-core/partmap/bsdlabel.c ../grub-core/partmap/dfly.c ../grub-core/script/function.c ../grub-core/script/lexer.c ../grub-core/script/main.c ../grub-core/script/script.c ../grub-core/script/argv.c ../grub-core/io/gzio.c ../grub-core/io/xzio.c ../grub-core/io/lzopio.c ../grub-core/kern/ia64/dl_helper.c ../grub-core/kern/arm/dl_helper.c ../grub-core/kern/arm64/dl_helper.c ../grub-core/lib/minilzo/minilzo.c ../grub-core/lib/xzembed/xz_dec_bcj.c ../grub-core/lib/xzembed/xz_dec_lzma2.c ../grub-core/lib/xzembed/xz_dec_stream.c ../util/misc.c ../grub-core/kern/command.c ../grub-core/kern/device.c ../grub-core/kern/disk.c ../grub-core/lib/disk.c ../util/getroot.c ../grub-core/osdep/unix/getroot.c ../grub-core/osdep/getroot.c ../grub-core/osdep/devmapper/getroot.c ../grub-core/osdep/relpath.c ../grub-core/kern/emu/hostdisk.c ../grub-core/osdep/devmapper/hostdisk.c ../grub-core/osdep/hostdisk.c ../grub-core/osdep/unix/hostdisk.c ../grub-core/osdep/exec.c ../grub-core/osdep/sleep.c ../grub-core/osdep/password.c ../grub-core/kern/emu/misc.c ../grub-core/kern/emu/mm.c ../grub-core/kern/env.c ../grub-core/kern/err.c ../grub-core/kern/file.c ../grub-core/kern/fs.c ../grub-core/kern/list.c ../grub-core/kern/misc.c ../grub-core/kern/partition.c ../grub-core/lib/crypto.c ../grub-core/disk/luks.c ../grub-core/disk/geli.c ../grub-core/disk/cryptodisk.c ../grub-core/disk/AFSplitter.c ../grub-core/lib/pbkdf2.c ../grub-core/commands/extcmd.c ../grub-core/lib/arg.c ../grub-core/disk/ldm.c ../grub-core/disk/diskfilter.c ../grub-core/partmap/gpt.c ../grub-core/partmap/msdos.c ../grub-core/fs/proc.c ../grub-core/fs/archelp.c > libgrub.pp || (rm -f libgrub.pp; exit 1) rm -f stamp-h1 touch ../config-util.h.in cd . && /bin/sh ./config.status config-util.h config.status: creating config-util.h In file included from ../include/grub/mm.h:25:0, from ../include/grub/disk.h:29, from ../include/grub/file.h:26, from ../grub-core/fs/btrfs.c:21: ./config.h:38:10: fatal error: ./config-util.h: No such file or directory #include ^~~~~~~~~~~~~~~ compilation terminated. make: *** [Makefile:13098: libgrub.pp] Error 1 This is because libgrub.pp is built with -DGRUB_UTIL=1, which means it'll try to include config-util.h, but a parallel make is actually building that file. I think. Reviewed-by: Daniel Kiper 2020-03-10 Peter Jones efi: Print more debug info in our module loader The function that searches the mods section base address does not have any debug information. Add some debugging outputs that could be useful. Reviewed-by: Daniel Kiper 2020-03-10 Peter Jones linux/getroot: Handle rssd storage device names The Micron PCIe SSDs Linux driver (mtip32xx) exposes block devices as /dev/rssd[a-z]+[0-9]*. Add support for these rssd device names. Reviewed-by: Daniel Kiper 2020-03-10 Julian Andres Klode smbios: Add a --linux argument to apply linux modalias-like filtering Linux creates modalias strings by filtering out non-ASCII, space, and colon characters. Provide an option that does the same filtering so people can create a modalias string in GRUB, and then match their modalias patterns against it. Reviewed-by: Daniel Kiper 2020-03-10 Mike Gilbert po: Fix replacement of %m in sed programs When running make dist, I hit this error: rm -f en@arabic.gmo && /usr/bin/gmsgfmt -c --statistics --verbose -o en@arabic.gmo en@arabic.po en@arabic.po:5312: 'msgstr' is not a valid C format string, unlike 'msgid'. Reason: The character that terminates the directive number 3 is not a valid conversion specifier. /usr/bin/gmsgfmt: found 1 fatal error This was caused by "%m" being replaced with foreign Unicode characters. For example: msgid "cannot rename the file %s to %s: %m" msgstr "ﺹﺎﻨﻧﻮﺗ ﺮﻌﻧﺎﻤﻋ ﺖﻬﻋ ﻒִﻴﻠﻋ %s ﺕﻭ %s: %ﻡ" Mimic the workaround used for "%s" by reversing the replacement of "%m" at the end of the sed programs. Reviewed-by: Daniel Kiper 2020-03-10 Colin Watson gettext: Restore patches to po/Makefile.in.in These were inadvertently lost during the conversion to Gnulib (gnulib: Upgrade Gnulib and switch to bootstrap tool; commit 35b909062). The files in po/gettext-patches/ can be imported using "git am" on top of the gettext tag corresponding to AM_GNU_GETTEXT_VERSION in configure.ac (currently 0.18.3). They handle translation of messages in shell files, make msgfmt output in little-endian format, and arrange to use @SHELL@ rather than /bin/sh. There were some changes solely for the purpose of distributing extra files; for ease of maintenance, I've added these to conf/Makefile.extra-dist instead. Fixes: https://savannah.gnu.org/bugs/?57298 Reviewed-by: Daniel Kiper 2020-02-28 Peter Jones misc: Make grub_strtol() "end" pointers have safer const qualifiers Currently the string functions grub_strtol(), grub_strtoul(), and grub_strtoull() don't declare the "end" pointer in such a way as to require the pointer itself or the character array to be immutable to the implementation, nor does the C standard do so in its similar functions, though it does require us not to change any of it. The typical declarations of these functions follow this pattern: long strtol(const char * restrict nptr, char ** restrict endptr, int base); Much of the reason for this is historic, and a discussion of that follows below, after the explanation of this change. (GRUB currently does not include the "restrict" qualifiers, and we name the arguments a bit differently.) The implementation is semantically required to treat the character array as immutable, but such accidental modifications aren't stopped by the compiler, and the semantics for both the callers and the implementation of these functions are sometimes also helped by adding that requirement. This patch changes these declarations to follow this pattern instead: long strtol(const char * restrict nptr, const char ** const restrict endptr, int base); This means that if any modification to these functions accidentally introduces either an errant modification to the underlying character array, or an accidental assignment to endptr rather than *endptr, the compiler should generate an error. (The two uses of "restrict" in this case basically mean strtol() isn't allowed to modify the character array by going through *endptr, and endptr isn't allowed to point inside the array.) It also means the typical use case changes to: char *s = ...; const char *end; long l; l = strtol(s, &end, 10); Or even: const char *p = str; while (p && *p) { long l = strtol(p, &p, 10); ... } This fixes 26 places where we discard our attempts at treating the data safely by doing: const char *p = str; long l; l = strtol(p, (char **)&ptr, 10); It also adds 5 places where we do: char *p = str; while (p && *p) { long l = strtol(p, (const char ** const)&p, 10); ... /* more calls that need p not to be pointer-to-const */ } While moderately distasteful, this is a better problem to have. With one minor exception, I have tested that all of this compiles without relevant warnings or errors, and that /much/ of it behaves correctly, with gcc 9 using 'gcc -W -Wall -Wextra'. The one exception is the changes in grub-core/osdep/aros/hostdisk.c , which I have no idea how to build. Because the C standard defined type-qualifiers in a way that can be confusing, in the past there's been a slow but fairly regular stream of churn within our patches, which add and remove the const qualifier in many of the users of these functions. This change should help avoid that in the future, and in order to help ensure this, I've added an explanation in misc.h so that when someone does get a compiler warning about a type error, they have the fix at hand. The reason we don't have "const" in these calls in the standard is purely anachronistic: C78 (de facto) did not have type qualifiers in the syntax, and the "const" type qualifier was added for C89 (I think; it may have been later). strtol() appears to date from 4.3BSD in 1986, which means it could not be added to those functions in the standard without breaking compatibility, which is usually avoided. The syntax chosen for type qualifiers is what has led to the churn regarding usage of const, and is especially confusing on string functions due to the lack of a string type. Quoting from C99, the syntax is: declarator: pointer[opt] direct-declarator direct-declarator: identifier ( declarator ) direct-declarator [ type-qualifier-list[opt] assignment-expression[opt] ] ... direct-declarator [ type-qualifier-list[opt] * ] ... pointer: * type-qualifier-list[opt] * type-qualifier-list[opt] pointer type-qualifier-list: type-qualifier type-qualifier-list type-qualifier ... type-qualifier: const restrict volatile So the examples go like: const char foo; // immutable object const char *foo; // mutable pointer to object char * const foo; // immutable pointer to mutable object const char * const foo; // immutable pointer to immutable object const char const * const foo; // XXX extra const keyword in the middle const char * const * const foo; // immutable pointer to immutable // pointer to immutable object const char ** const foo; // immutable pointer to mutable pointer // to immutable object Making const left-associative for * and right-associative for everything else may not have been the best choice ever, but here we are, and the inevitable result is people using trying to use const (as they should!), putting it at the wrong place, fighting with the compiler for a bit, and then either removing it or typecasting something in a bad way. I won't go into describing restrict, but its syntax has exactly the same issue as with const. Anyway, the last example above actually represents the *behavior* that's required of strtol()-like functions, so that's our choice for the "end" pointer. Reviewed-by: Daniel Kiper 2020-02-28 Mike Gilbert build: Disable PIE in TARGET_CCASFLAGS if needed PIE should be disabled in assembly sources as well, or else GRUB will fail to boot. Bug: https://bugs.gentoo.org/667852 Reviewed-by: Daniel Kiper Tested-by: John Paul Adrian Glaubitz 2020-02-28 Mike Gilbert build: Move TARGET_* assignments earlier On a 32-bit SPARC userland, configure fails to compile assembly and the build fails: checking for options to compile assembly... configure: error: could not compile assembly config.log shows: asm-tests/sparc64.S: Assembler messages: asm-tests/sparc64.S:5: Error: Architecture mismatch on "lduw [%o4+4],%o4". asm-tests/sparc64.S:5: (Requires v9|v9a|v9b|v9c|v9d|v9e|v9v|v9m|m8; requested architecture is sparclite.) asm-tests/sparc64.S:7: Error: Architecture mismatch on "stw %o5,[%o3]". asm-tests/sparc64.S:7: (Requires v9|v9a|v9b|v9c|v9d|v9e|v9v|v9m|m8; requested architecture is sparclite.) asm-tests/sparc64.S:8: Error: Architecture mismatch on "bne,pt %icc,1b ,pt %icc,1b". asm-tests/sparc64.S:8: (Requires v9|v9a|v9b|v9c|v9d|v9e|v9v|v9m|m8; requested architecture is sparclite.) Simply moving these blocks earlier in configure.ac is sufficient to ensure that the tests are executed with the appropriate flags (specifically -m64 in this case). Bug: https://bugs.gentoo.org/667850 Reviewed-by: Daniel Kiper Tested-by: John Paul Adrian Glaubitz 2020-02-28 Patrick Steinhardt luks2: Add missing newline to debug message The debug message printed when decryption with a keyslot fails is missing its trailing newline. Add it to avoid mangling it with subsequent output. Reviewed-by: Daniel Kiper 2020-02-18 Michael Chang verifiers: Fix calling uninitialized function pointer The necessary check for NULL before use of function ver->close is not taking place in the failure path. This patch simply adds the missing check and fixes the problem that GRUB hangs indefinitely after booting rogue image without valid signature if secure boot is turned on. Now it displays like this for booting rogue UEFI image: error: bad shim signature error: you need to load the kernel first Press any key to continue... and then you can go back to boot menu by pressing any key or after a few seconds expired. Reviewed-by: Javier Martinez Canillas Reviewed-by: Daniel Kiper 2020-02-18 Peter Jones grub-editenv: Make grub-editenv chase symlinks including those across devices The grub-editenv create command will wrongly overwrite /boot/grub2/grubenv with a regular file if grubenv is a symbolic link. But instead, it should create a new file in the path the symlink points to. This lets /boot/grub2/grubenv be a symlink to /boot/efi/EFI/fedora/grubenv even when they're different mount points, which allows grub2-editenv to be the same across platforms (i.e. UEFI vs BIOS). For example, in Fedora the GRUB EFI builds have prefix set to /EFI/fedora (on the EFI System Partition), but for BIOS machine it'll be /boot/grub2 (which may or may not be its own mountpoint). With this patch, on EFI machines we can make /boot/grub2/grubenv a symlink to /boot/efi/EFI/fedora/grubenv, and the same copy of grub-set-default will work on both kinds of systems. Windows doesn't implement a readlink primitive, so the current behaviour is maintained for this operating system. Reviewed-by: Adam Jackson Reviewed-by: Daniel Kiper 2020-02-18 Peter Jones grub-editenv: Add grub_util_readlink() Currently grub-editenv and related tools are not able to follow symbolic links when finding their config file. For example the grub-editenv create command will wrongly overwrite a symlink in /boot/grub2/grubenv with a new regular file, instead of creating a file in the path the symlink points to. A following patch will change that and add support in grub-editenv to follow symbolic links when finding the grub environment variables file. Add a grub_util_readlink() helper function that is just a wrapper around the platform specific function to read the value of a symbolic link. This helper function will be used by the following patch for grub-editenv. The helper function is not added for Windows, since this operating system doesn't have a primitive to read the contents of a symbolic link. Reviewed-by: Adam Jackson Reviewed-by: Daniel Kiper 2020-02-18 Robert Marshall docs: Update info with grub.cfg netboot selection order Add documentation to the GRUB manual that specifies the order netboot clients use to select a GRUB configuration file. Also explain that the feature is enabled by default but can be disabled by setting the "feature_net_search_cfg" environment variable to "n" in an embedded configuration file. Reviewed-by: Daniel Kiper 2020-02-18 Paulo Flabiano Smorigo normal/main: Search for specific config files for netboot This patch implements a search for a specific configuration when the config file is on a remoteserver. It uses the following order: 1) DHCP client UUID option. 2) MAC address (in lower case hexadecimal with dash separators); 3) IP (in upper case hexadecimal) or IPv6; 4) The original grub.cfg file. This procedure is similar to what is used by pxelinux and yaboot: http://www.syslinux.org/wiki/index.php/PXELINUX#config It is enabled by default but can be disabled by setting the environment variable "feature_net_search_cfg" to "n" in an embedded configuration. Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=873406 Reviewed-by: Daniel Kiper 2020-02-18 Paulo Flabiano Smorigo net/dhcp: Set net__client{id, uuid} variables from DHCP options This patch sets a net__clientid and net__clientuuid GRUB environment variables, using the DHCP client ID and UUID options if these are found. In the same way than net__