linux-kernel - [GRUB PATCH 1/1] loader/i386/linux: Fix an underflow in the setup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20191202172939.29271-1-daniel.kiper@oracle.com>
Date:   Mon,  2 Dec 2019 18:29:39 +0100
From:   Daniel Kiper <daniel.kiper@...cle.com>
To:     grub-devel@....org, linux-kernel@...r.kernel.org, x86@...nel.org
Cc:     bp@...en8.de, eric.snowberg@...cle.com, hpa@...or.com,
        kanth.ghatraju@...cle.com, konrad.wilk@...cle.com,
        mingo@...hat.com, phcoder@...il.com, rdunlap@...radead.org,
        ross.philipson@...cle.com
Subject: [GRUB PATCH 1/1] loader/i386/linux: Fix an underflow in the setup_header length calculation

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@...cle.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