Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751913AbdGGK5D (ORCPT ); Fri, 7 Jul 2017 06:57:03 -0400 Received: from mail-wr0-f177.google.com ([209.85.128.177]:34710 "EHLO mail-wr0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750984AbdGGK5C (ORCPT ); Fri, 7 Jul 2017 06:57:02 -0400 Date: Fri, 7 Jul 2017 11:56:58 +0100 From: Matt Fleming To: Baoquan He Cc: Naoya Horiguchi , Kees Cook , LKML , "x86@kernel.org" , Thomas Gleixner , "H. Peter Anvin" , Ingo Molnar , "izumi.taku@jp.fujitsu.com" , Thomas Garnier , "fanc.fnst@cn.fujitsu.com" , Junichi Nomura , Ard Biesheuvel Subject: Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice Message-ID: <20170707105658.GA9917@codeblueprint.co.uk> References: <20170706083106.GA21796@hori1.linux.bs1.fc.nec.co.jp> <20170706145727.GB3080@codeblueprint.co.uk> <20170707030759.GA2343@x1> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170707030759.GA2343@x1> User-Agent: Mutt/1.5.24+42 (6e565710a064) (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1839 Lines: 45 On Fri, 07 Jul, at 11:07:59AM, Baoquan He wrote: > On 07/06/17 at 03:57pm, Matt Fleming wrote: > > On Thu, 06 Jul, at 08:31:07AM, Naoya Horiguchi wrote: > > > + for (i = 0; i < nr_desc; i++) { > > > + md = (efi_memory_desc_t *)(pmap + (i * e->efi_memdesc_size)); > > > + > > > + /* > > > + * EFI_BOOT_SERVICES_{CODE|DATA} are avoided because boot > > > + * services regions could be accessed after ExitBootServices() > > > + * due to the workaround for buggy firmware. > > > + */ > > > + if (!(md->type == EFI_LOADER_CODE || > > > + md->type == EFI_LOADER_DATA || > > > + md->type == EFI_CONVENTIONAL_MEMORY)) > > > + continue; > > > > Wouldn't it make more sense to *only* use EFI_CONVENTIONAL_MEMORY? > > > > You can't re-use EFI_LOADER_* regions because the kaslr code is run so > > early in boot that you've no idea if data the kernel will need is in > > those EFI_LOADER_* regions. > > > > For example, we pass struct setup_data objects inside of > > EFI_LOADER_DATA regions. > > It doesn't matter because we have tried to avoid those memory setup_data > resides in in mem_avoid_overlap(). Here discarding EFI_LOADER_* could > discard the whole regions while setup_data could occupy small part of > them. What about the GDT that we allocate in the x86 EFI boot stub as EFI_LOADER_DATA? Are there functions to avoid that too? What about any future uses we add? Who's going to remember to patch the kaslr code which now duplicates some of the EFI memory map logic? All of these problems can avoided if you just stick with EFI_CONVENTIONAL_MEMORY. Honestly, how much memory do we expect to waste if we ignore EFI_LOADER_* regions? Also, the fact that you're referencing EFI-specific boot quirks in the kaslr code should be a massive red flag that you're playing with the innards of the EFI subsystem.