Messages in this thread | | | Date | Sun, 08 Dec 2019 14:47:22 -0800 | Subject | Re: [GRUB PATCH 1/1] loader/i386/linux: Fix an underflow in the setup_header length calculation | From | hpa@zytor ... |
| |
On December 8, 2019 2:43:57 PM PST, Daniel Kiper <daniel.kiper@oracle.com> wrote: >Hmmm... Nobody cares? Strange... > >Adding 3mdeb folks... > >Daniel > >On Mon, Dec 02, 2019 at 06:29:39PM +0100, Daniel Kiper wrote: >> 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.kiper@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
Please fix these problems. It is a huge headache for us in the kennel community. -- Sent from my Android device with K-9 Mail. Please excuse my brevity.
| |