linux-kernel - Re: [PATCH v3 1/2] x86/efi: don't allocate memmap through memblock after mm

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 5 Jan 2017 13:30:51 -0800
From:   Dan Williams <dan.j.williams@...el.com>
To:     Nicolai Stange <nicstange@...il.com>
Cc:     Ingo Molnar <mingo@...nel.org>,
        Matt Fleming <matt@...eblueprint.co.uk>,
        Thomas Gleixner <tglx@...utronix.de>,
        "H. Peter Anvin" <hpa@...or.com>,
        "x86@...nel.org" <x86@...nel.org>,
        Mika Penttilä <mika.penttila@...tfour.com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Dave Young <dyoung@...hat.com>, linux-efi@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 1/2] x86/efi: don't allocate memmap through memblock
 after mm_init()

On Thu, Jan 5, 2017 at 4:51 AM, Nicolai Stange <nicstange@...il.com> wrote:
> With commit 4bc9f92e64c8 ("x86/efi-bgrt: Use efi_mem_reserve() to avoid
> copying image data"), efi_bgrt_init() calls into the memblock allocator
> through efi_mem_reserve() => efi_arch_mem_reserve() *after* mm_init()
> has been called.
>
> Indeed, KASAN reports a bad read access later on in
> efi_free_boot_services():
>
>   BUG: KASAN: use-after-free in efi_free_boot_services+0xae/0x24c
>             at addr ffff88022de12740
>   Read of size 4 by task swapper/0/0
>   page:ffffea0008b78480 count:0 mapcount:-127
>   mapping:          (null) index:0x1 flags: 0x5fff8000000000()
>   [...]
>   Call Trace:
>    dump_stack+0x68/0x9f
>    kasan_report_error+0x4c8/0x500
>    kasan_report+0x58/0x60
>    __asan_load4+0x61/0x80
>    efi_free_boot_services+0xae/0x24c
>    start_kernel+0x527/0x562
>    x86_64_start_reservations+0x24/0x26
>    x86_64_start_kernel+0x157/0x17a
>    start_cpu+0x5/0x14
>
> The instruction at the given address is the first read from the memmap's
> memory, i.e. the read of md->type in efi_free_boot_services().
>
> Note that the writes earlier in efi_arch_mem_reserve() don't splat because
> they're done through early_memremap()ed addresses.
>
> So, after memblock is gone, allocations should be done through the "normal"
> page allocator. Introduce a helper, efi_memmap_alloc() for this. Use
> it from efi_arch_mem_reserve(), efi_free_boot_services() and, for the sake
> of consistency, from efi_fake_memmap() as well.
>
> Note that for the latter, the memmap allocations cease to be page aligned.
> This isn't needed though.
>
> Fixes: 4bc9f92e64c8 ("x86/efi-bgrt: Use efi_mem_reserve() to avoid copying image data")
> Signed-off-by: Nicolai Stange <nicstange@...il.com>
> ---
> Applicable to next-20170105.
> Boot-tested with an efi_fake_mem= parameter on x86_64.
>
> v2 of this patch got a
>
>   Tested-by: Dan Williams <dan.j.williams@...el.com>
>
>
> Changes to v2:
>  - Use the new efi_memmap_alloc() from efi_fake_memmap(), too.
>  - Update commit message accordingly.
>
>  arch/x86/platform/efi/quirks.c  |  4 ++--
>  drivers/firmware/efi/fake_mem.c |  3 +--
>  drivers/firmware/efi/memmap.c   | 38 ++++++++++++++++++++++++++++++++++++++
>  include/linux/efi.h             |  1 +
>  4 files changed, 42 insertions(+), 4 deletions(-)

v3 works as well:

Tested-by: Dan Williams <dan.j.williams@...el.com>