Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751964AbdLFODG (ORCPT ); Wed, 6 Dec 2017 09:03:06 -0500 Received: from mail-io0-f196.google.com ([209.85.223.196]:46849 "EHLO mail-io0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751544AbdLFODD (ORCPT ); Wed, 6 Dec 2017 09:03:03 -0500 X-Google-Smtp-Source: AGs4zMbhz48DZoom55XzHVXKb6xqtn3yK9iD7Ld48UFFjTG5YRJNg2WgD1JrJnli5+IqaEqY95Kz3tbyqcGbF5ATFRY= MIME-Version: 1.0 In-Reply-To: <20171206132714.GA31186@arm.com> References: <1512563739-25239-1-git-send-email-will.deacon@arm.com> <1512563739-25239-21-git-send-email-will.deacon@arm.com> <20171206132714.GA31186@arm.com> From: Ard Biesheuvel Date: Wed, 6 Dec 2017 14:03:01 +0000 Message-ID: Subject: Re: [PATCH v3 20/20] arm64: kaslr: Put kernel vectors address in separate data page To: Will Deacon Cc: "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , Catalin Marinas , Mark Rutland , Stephen Boyd , Dave Hansen , Kees Cook , Mark Salter , Laura Abbott , "tglx@linutronix.de" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4476 Lines: 111 On 6 December 2017 at 13:27, Will Deacon wrote: > Hi Ard, > > On Wed, Dec 06, 2017 at 12:59:40PM +0000, Ard Biesheuvel wrote: >> On 6 December 2017 at 12:35, Will Deacon wrote: >> > The literal pool entry for identifying the vectors base is the only piece >> > of information in the trampoline page that identifies the true location >> > of the kernel. >> > >> > This patch moves it into its own page, which is only mapped by the full >> > kernel page table, which protects against any accidental leakage of the >> > trampoline contents. > > [...] > >> > @@ -1073,6 +1079,11 @@ END(tramp_exit_compat) >> > >> > .ltorg >> > .popsection // .entry.tramp.text >> > +#ifdef CONFIG_RANDOMIZE_BASE >> > + .pushsection ".entry.tramp.data", "a" // .entry.tramp.data >> > + .quad vectors >> > + .popsection // .entry.tramp.data >> >> This does not need to be in a section of its own, and doesn't need to >> be padded to a full page. >> >> If you just stick this in .rodata but align it to page size, you can >> just map whichever page it ends up in into the TRAMP_DATA fixmap slot >> (which is a r/o mapping anyway). You could then drop most of the >> changes below. And actually, I'm not entirely sure whether it still >> makes sense then to do this only for CONFIG_RANDOMIZE_BASE. > > Good point; I momentarily forgot this was in the kernel page table anyway. > How about something like the diff below merged on top (so this basically > undoes a bunch of the patch)? > Yes, that looks much better. > I'd prefer to keep the CONFIG_RANDOMIZE_BASE dependency, at least for now. > Fair enough. > --->8 > > diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S > index a70c6dd2cc19..031392ee5f47 100644 > --- a/arch/arm64/kernel/entry.S > +++ b/arch/arm64/kernel/entry.S > @@ -1080,9 +1080,12 @@ END(tramp_exit_compat) > .ltorg > .popsection // .entry.tramp.text > #ifdef CONFIG_RANDOMIZE_BASE > - .pushsection ".entry.tramp.data", "a" // .entry.tramp.data > + .pushsection ".rodata", "a" > + .align PAGE_SHIFT > + .globl __entry_tramp_data_start > +__entry_tramp_data_start: > .quad vectors > - .popsection // .entry.tramp.data > + .popsection // .rodata > #endif /* CONFIG_RANDOMIZE_BASE */ > #endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */ > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > index 976109b3ae51..27cf9be20a00 100644 > --- a/arch/arm64/kernel/vmlinux.lds.S > +++ b/arch/arm64/kernel/vmlinux.lds.S > @@ -64,21 +64,8 @@ jiffies = jiffies_64; > *(.entry.tramp.text) \ > . = ALIGN(PAGE_SIZE); \ > VMLINUX_SYMBOL(__entry_tramp_text_end) = .; > -#ifdef CONFIG_RANDOMIZE_BASE > -#define TRAMP_DATA \ > - .entry.tramp.data : { \ > - . = ALIGN(PAGE_SIZE); \ > - VMLINUX_SYMBOL(__entry_tramp_data_start) = .; \ > - *(.entry.tramp.data) \ > - . = ALIGN(PAGE_SIZE); \ > - VMLINUX_SYMBOL(__entry_tramp_data_end) = .; \ > - } > -#else > -#define TRAMP_DATA > -#endif /* CONFIG_RANDOMIZE_BASE */ > #else > #define TRAMP_TEXT > -#define TRAMP_DATA > #endif /* CONFIG_UNMAP_KERNEL_AT_EL0 */ > > /* > @@ -150,7 +137,6 @@ SECTIONS > RO_DATA(PAGE_SIZE) /* everything from this point to */ > EXCEPTION_TABLE(8) /* __init_begin will be marked RO NX */ > NOTES > - TRAMP_DATA > > . = ALIGN(SEGMENT_ALIGN); > __init_begin = .; > @@ -268,10 +254,6 @@ ASSERT(__hibernate_exit_text_end - (__hibernate_exit_text_start & ~(SZ_4K - 1)) > #ifdef CONFIG_UNMAP_KERNEL_AT_EL0 > ASSERT((__entry_tramp_text_end - __entry_tramp_text_start) == PAGE_SIZE, > "Entry trampoline text too big") > -#ifdef CONFIG_RANDOMIZE_BASE > -ASSERT((__entry_tramp_data_end - __entry_tramp_data_start) == PAGE_SIZE, > - "Entry trampoline data too big") > -#endif > #endif > /* > * If padding is applied before .head.text, virt<->phys conversions will fail.