On Tue, Mar 02, 2021 at 07:00:08PM +0100, Daniel Kiper wrote: > From: Marco A Benatto <mbena...@redhat.com> > > 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.
I've had reports in Debian that the i386-pc image no longer fits in the MBR in some configurations (e.g. https://bugs.debian.org/984488, https://bugs.debian.org/985374). (Yes, I know, MBR is awful and even people who have to use it should put the first partition at more like 1MiB rather than 63 sectors, but it isn't practical for non-experts to fix up existing systems without a complete reinstallation, so breaking this in a security update is pretty bad.) Since I suspected that a lot of this was due to organic growth of the image as the security patch series made various bits of code more careful, I wrote a script to build each revision along the upstream changes from Debian version 2.04-15 to 2.04-16 and build an image with the following modules, extracted from one of those bug reports: ext2 part_msdos diskfilter mdraid09 biosdisk. Here's a report of the resulting image sizes and commits: 30884 2bd6855d2 grub-install: Fix inverted test for NLS enabled when copying locales 31427 0d324ad1b verifiers: Move verifiers API to kernel image 31446 6e14c57c6 kern: Add lockdown support 31446 f1d70c97b kern/lockdown: Set a variable if the GRUB is locked down 31446 71b48a193 efi: Lockdown the GRUB when the UEFI Secure Boot is enabled 31446 3d8afd579 efi: Use grub_is_lockdown() instead of hardcoding a disabled modules list 31446 c3037730d acpi: Don't register the acpi command when locked down 31446 5d58cce5c mmap: Don't register cutmem and badram commands when lockdown is enforced 31446 22f08600d commands: Restrict commands that can load BIOS or DT blobs when locked down 31446 bf939ef4e commands/setpci: Restrict setpci command when locked down 31446 ad9d55e50 commands/hdparm: Restrict hdparm command when locked down 31446 13a1fa9c1 gdb: Restrict GDB access when locked down 31446 b1e1dd471 loader/xnu: Don't allow loading extension and packages when locked down 31446 9042c1bc8 docs: Document the cutmem command 31452 9e6b789fa dl: Only allow unloading modules that are not dependencies 31452 d26f10df9 usb: Avoid possible out-of-bound accesses caused by malicious devices 31452 a993a2006 mmap: Fix memory leak when iterating over mapped memory 31452 60709e32e net/net: Fix possible dereference to of a NULL pointer 31452 118fe4df3 net/tftp: Fix dangling memory pointer 31473 967b95c4e kern/parser: Fix resource leak if argc == 0 31473 42b46cb07 kern/efi: Fix memory leak on failure 31473 10f42aeff kern/efi/mm: Fix possible NULL pointer dereference 31473 ad3b3b125 gnulib/regexec: Resolve unused variable 31473 a0b08bad3 gnulib/regcomp: Fix uninitialized token structure 31473 3131d3ff8 gnulib/argp-help: Fix dereference of a possibly NULL state 31473 dc28cd75d gnulib/regexec: Fix possible null-dereference 31473 711dd9d97 gnulib/regcomp: Fix uninitialized re_token 31473 28314f6c1 io/lzopio: Resolve unnecessary self-assignment errors 31473 f4eb2c3dd zstd: Initialize seq_t structure fully 31482 6d368ec03 kern/partition: Check for NULL before dereferencing input string 31482 e743b06fc disk/ldm: Make sure comp data is freed before exiting from make_vg() 31482 af94bf626 disk/ldm: If failed then free vg variable too 31482 8e43b154c disk/ldm: Fix memory leak on uninserted lv references 31482 0beb60002 disk/cryptodisk: Fix potential integer overflow 31482 20ddfae56 hfsplus: Check that the volume name length is valid 31482 d8fa680fe zfs: Fix possible negative shift operation 31482 1b80d2dde zfs: Fix resource leaks while constructing path 31482 2b07acad0 zfs: Fix possible integer overflows 31482 0283863c7 zfsinfo: Correct a check for error allocating memory 31482 ad663e4ea affs: Fix memory leaks 31482 8d9e05f24 libgcrypt/mpi: Fix possible unintended sign extension 31482 3120a6835 libgcrypt/mpi: Fix possible NULL dereference 31482 6d38008dd syslinux: Fix memory leak while parsing 31482 06f86bc0d normal/completion: Fix leaking of memory when processing a completion 31482 e31e8ecbc commands/hashsum: Fix a memory leak 31482 74d544182 video/efi_gop: Remove unnecessary return value of grub_video_gop_fill_mode_info() 31482 e07f13cfa video/fb/fbfill: Fix potential integer overflow 31482 fffc476df video/fb/video_fb: Fix multiple integer overflows 31482 786656dc8 video/fb/video_fb: Fix possible integer overflow 31482 bf3df4eeb video/readers/jpeg: Test for an invalid next marker reference from a jpeg file 31482 f9b9c56e2 gfxmenu/gui_list: Remove code that coverity is flagging as dead 31482 11cf998c2 loader/bsd: Check for NULL arg up-front 31482 d311599e4 loader/xnu: Fix memory leak 31482 986de6735 loader/xnu: Free driverkey data when an error is detected in grub_xnu_writetree_toheap() 31482 f851813cd loader/xnu: Check if pointer is NULL before using it 31482 4f7bde3ab util/grub-install: Fix NULL pointer dereferences 31482 fd0e3f964 util/grub-editenv: Fix incorrect casting of a signed value 31482 d86e80fe0 util/glue-efi: Fix incorrect use of a possibly negative value 31482 6a84527d4 script/execute: Fix NULL dereference in grub_script_execute_cmdline() 31482 f785f176a commands/ls: Require device_name is not NULL before printing 31482 12f5e77dc script/execute: Avoid crash when using "$#" outside a function scope 31482 82446d230 lib/arg: Block repeated short options that require an argument 31482 121811a98 script/execute: Don't crash on a "for" loop with no items 31482 3c5bfae9e commands/menuentry: Fix quoting in setparams_prefix() 31474 abdc1e40a kern/misc: Always set *end in grub_strtoull() 31474 b189d92cb video/readers/jpeg: Catch files with unsupported quantization or Huffman tables 31474 7b5a6dc77 video/readers/jpeg: Catch OOB reads/writes in grub_jpeg_decode_du() 31474 14cc3bde4 video/readers/jpeg: Don't decode data before start of stream 31474 0013a6fa9 term/gfxterm: Don't set up a font with glyphs that are too big 31508 2832f9ed4 fs/fshelp: Catch impermissibly large block sizes in read helper 31508 0a8501280 fs/hfsplus: Don't fetch a key beyond the end of the node 31508 1632d4751 fs/hfsplus: Don't use uninitialized data on corrupt filesystems 31508 3dbfcb563 fs/hfs: Disable under lockdown 31508 0e5a7bb86 fs/sfs: Fix over-read of root object name 31508 4089be10f fs/jfs: Do not move to leaf level if name length is negative 31508 2de73233c fs/jfs: Limit the extents that getblk() can consider 31508 751f1ad0b fs/jfs: Catch infinite recursion 31508 64b8e6ab3 fs/nilfs2: Reject too-large keys 31508 e5b544089 fs/nilfs2: Don't search children if provided number is too large 31508 6187f84c0 fs/nilfs2: Properly bail on errors in grub_nilfs2_btree_node_lookup() 31508 ab6d7615d io/gzio: Bail if gzio->tl/td is NULL 31508 b3c863498 io/gzio: Add init_dynamic_block() clean up if unpacking codes fails 31508 fd737860d io/gzio: Catch missing values in huft_build() and bail 31508 2df30bf33 io/gzio: Zero gzio->tl/td in init_dynamic_block() if huft_build() fails 31508 64eb78f6c disk/lvm: Don't go beyond the end of the data we read from disk 31508 b448c78a1 disk/lvm: Don't blast past the end of the circular metadata buffer 31508 ff49d996d disk/lvm: Bail on missing PV list 31508 87625cadf disk/lvm: Do not crash if an expected string is not found 31508 aed3e7107 disk/lvm: Do not overread metadata 31508 3fc149f18 disk/lvm: Sanitize rlocn->offset to prevent wild read 31508 e8f1ae648 disk/lvm: Do not allow a LV to be it's own segment's node's LV 31508 bd87498f4 fs/btrfs: Validate the number of stripes/parities in RAID5/6 31508 99572884b fs/btrfs: Squash some uninitialized reads 31529 3f1acab9c kern/parser: Fix a memory leak 31529 7feb878e8 kern/parser: Introduce process_char() helper 31556 3ab27438b kern/parser: Introduce terminate_arg() helper 31541 782fb1971 kern/parser: Refactor grub_parser_split_cmdline() cleanup 31698 fc938b31c kern/buffer: Add variable sized heap buffer 31826 596d36219 kern/parser: Fix a stack buffer overflow 31826 149524eaa kern/efi: Add initial stack protector implementation 31826 039725fe7 util/mkimage: Remove unused code to add BSS section 31826 0a6005c74 util/mkimage: Use grub_host_to_target32() instead of grub_cpu_to_le32() 31826 cf179bd80 util/mkimage: Always use grub_host_to_target32() to initialize PE stack and heap stuff 31826 2aa0ef41c util/mkimage: Unify more of the PE32 and PE32+ header set-up 31826 1aa253780 util/mkimage: Reorder PE optional header fields set-up 31826 99f02aab1 util/mkimage: Improve data_size value calculation 31826 1c810627e util/mkimage: Refactor section setup to use a helper 31826 26ff81d1e util/mkimage: Add an option to import SBAT metadata into a .sbat section 31826 2564455e7 grub-install-common: Add --sbat option 31826 55c7de529 kern/misc: Split parse_printf_args() into format parsing and va_list handling 31787 c8bc04397 kern/misc: Add STRING type for internal printf() format handling 32060 834e5d238 kern/misc: Add function to check printf() format against expected format 32060 c652b0e86 gfxmenu/gui: Check printf() format in the gui_progress_bar and gui_label 32060 9cd32c576 kern/mm: Fix grub_debug_calloc() compilation error I believe the practical threshold is 62 512-byte sectors, i.e. 31744 bytes. As you can see, the biggest single change was induced by this patch, which moves the verifiers API into the kernel image. Makes sense. Is there anything we can do about this? I'm a little confused why this change had to be made in this way. grub_load_modules is called pretty early during kernel initialization, and it initializes all embedded modules. Wouldn't it have been sufficient to leave verifiers as a module and simply include that module in all UEFI-platform images? If that wouldn't have worked for some reason, then perhaps it would be possible to restructure things a bit more so that we could leave the verifiers API as a module on i386-pc, e.g. by moving it back to grub-core/commands/verifiers.c and having conditional code that either registers/unregisters the filter in a module or registers it at kernel startup, depending on the platform. It wouldn't be especially pretty, but I think we could tolerate that for the sake of fixing this regression. Thanks, -- Colin Watson (he/him) [cjwat...@debian.org] _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
