Recent work around x86 Linux kernel loader revealed an underflow in the setup_header length calculation and another related issue. Both lead to the memory overwrite and later machine crash.
Currently when the GRUB copies the setup_header into the linux_params (struct boot_params, traditionally known as "zero page") it assumes the setup_header size as sizeof(linux_i386_kernel_header/lh). This is incorrect. It should use the value calculated accordingly to the Linux kernel boot protocol. Otherwise in case of pretty old kernel, to be exact Linux kernel boot protocol, the GRUB may write more into linux_params than it was expected to. Fortunately this is not very big issue. Though it has to be fixed. However, there is also an underflow which is grave. It happens when sizeof(linux_i386_kernel_header/lh) > "real size of the setup_header". Then len value wraps around and grub_file_read() reads whole kernel into the linux_params overwriting memory past it. This leads to the GRUB memory allocator breakage and finally to its crash during boot. The patch fixes both issues. Additionally, it moves the code not related to grub_memset(linux_params)/grub_memcpy(linux_params)/grub_file_read(linux_params) section outside of it to not confuse the reader. Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com> --- grub-core/loader/i386/linux.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c index d0501e229..ee95cd374 100644 --- a/grub-core/loader/i386/linux.c +++ b/grub-core/loader/i386/linux.c @@ -761,17 +761,12 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)), goto fail; grub_memset (&linux_params, 0, sizeof (linux_params)); - grub_memcpy (&linux_params.setup_sects, &lh.setup_sects, sizeof (lh) - 0x1F1); - - linux_params.code32_start = prot_mode_target + lh.code32_start - GRUB_LINUX_BZIMAGE_ADDR; - linux_params.kernel_alignment = (1 << align); - linux_params.ps_mouse = linux_params.padding10 = 0; /* * The Linux 32-bit boot protocol defines the setup header end * to be at 0x202 + the byte value at 0x201. */ - len = 0x202 + *((char *) &linux_params.jump + 1); + len = 0x202 + *((char *) &lh.jump + 1); /* Verify the struct is big enough so we do not write past the end. */ if (len > (char *) &linux_params.edd_mbr_sig_buffer - (char *) &linux_params) { @@ -779,10 +774,13 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)), goto fail; } + grub_memcpy (&linux_params.setup_sects, &lh.setup_sects, len - 0x1F1); + /* We've already read lh so there is no need to read it second time. */ len -= sizeof(lh); - if (grub_file_read (file, (char *) &linux_params + sizeof (lh), len) != len) + if ((len > 0) && + (grub_file_read (file, (char *) &linux_params + sizeof (lh), len) != len)) { if (!grub_errno) grub_error (GRUB_ERR_BAD_OS, N_("premature end of file %s"), @@ -790,6 +788,9 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)), goto fail; } + linux_params.code32_start = prot_mode_target + lh.code32_start - GRUB_LINUX_BZIMAGE_ADDR; + linux_params.kernel_alignment = (1 << align); + linux_params.ps_mouse = linux_params.padding10 = 0; linux_params.type_of_loader = GRUB_LINUX_BOOT_LOADER_TYPE; /* These two are used (instead of cmd_line_ptr) by older versions of Linux, -- 2.11.0 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel