From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S938958AbdAEVbe (ORCPT ); Thu, 5 Jan 2017 16:31:34 -0500 Received: from mail-oi0-f51.google.com ([209.85.218.51]:36706 "EHLO mail-oi0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935063AbdAEVax (ORCPT ); Thu, 5 Jan 2017 16:30:53 -0500 MIME-Version: 1.0 In-Reply-To: <20170105125130.2815-1-nicstange@gmail.com> References: <20170105125130.2815-1-nicstange@gmail.com> From: Dan Williams Date: Thu, 5 Jan 2017 13:30:51 -0800 Message-ID: Subject: Re: [PATCH v3 1/2] x86/efi: don't allocate memmap through memblock after mm_init() To: Nicolai Stange Cc: Ingo Molnar , Matt Fleming , Thomas Gleixner , "H. Peter Anvin" , "x86@kernel.org" , =?UTF-8?Q?Mika_Penttil=C3=A4?= , Ard Biesheuvel , Dave Young , linux-efi@vger.kernel.org, "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jan 5, 2017 at 4:51 AM, Nicolai Stange 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 > --- > 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 > > > 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