linux-kernel - Re: [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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <74fdf9ed-814b-0fc8-d405-79eb1011b9ee@redhat.com>
Date:   Fri, 13 Dec 2019 13:57:26 +0100
From:   Javier Martinez Canillas <fmartine@...hat.com>
To:     The development of GNU GRUB <grub-devel@....org>,
        Daniel Kiper <daniel.kiper@...cle.com>,
        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: Re: [GRUB PATCH 1/1] loader/i386/linux: Fix an underflow in the
 setup_header length calculation

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

The patch looks good to me.

Reviewed-by: Javier Martinez Canillas <javierm@...hat.com>

Best regards,
-- 
Javier Martinez Canillas
Software Engineer - Desktop Hardware Enablement
Red Hat