Messages in this thread | | | Subject | Re: [GRUB PATCH 1/1] loader/i386/linux: Fix an underflow in the setup_header length calculation | From | Javier Martinez Canillas <> | Date | Fri, 13 Dec 2019 13:57:26 +0100 |
| |
Hello Daniel,
On 12/2/19 6:29 PM, 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. >
Maybe you should add the following tag?
Fixes: e683cfb0cf5 ("loader/i386/linux: Calculate the setup_header length")
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> > ---
The patch looks good to me.
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Best regards, -- Javier Martinez Canillas Software Engineer - Desktop Hardware Enablement Red Hat
| |