Date: Thu, 14 Apr 2016 10:56:04 -0700 From: Kees Cook <keescook@...omium.org> To: Baoquan He <bhe@...hat.com> Cc: Ingo Molnar <mingo@...nel.org>, Ingo Molnar <mingo@...hat.com>, LKML <linux-kernel@...r.kernel.org>, Yinghai Lu <yinghai@...nel.org>, "H. Peter Anvin" <hpa@...or.com>, Borislav Petkov <bp@...en8.de>, Vivek Goyal <vgoyal@...hat.com>, Andy Lutomirski <luto@...nel.org>, lasse.collin@...aani.org, Andrew Morton <akpm@...ux-foundation.org>, Dave Young <dyoung@...hat.com>, "kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com> Subject: Re: [PATCH v4 00/20] x86, boot: kaslr cleanup and 64bit kaslr support On Thu, Apr 14, 2016 at 8:06 AM, Baoquan He <bhe@...hat.com> wrote: > On 04/13/16 at 11:02pm, Kees Cook wrote: >> On Wed, Apr 13, 2016 at 7:11 AM, Kees Cook <keescook@...omium.org> wrote: >> > On Wed, Apr 13, 2016 at 3:19 AM, Ingo Molnar <mingo@...nel.org> wrote: >> >> >> >> * Kees Cook <keescook@...omium.org> wrote: >> >> >> >>> FWIW, I've also had this tree up in my git branches, and the 0day >> >>> tester hasn't complained at all about it in the last two weeks. I'd >> >>> really like to see this in -next to fix the >4G (mainly kexec) issues >> >>> and get us to feature parity with the arm64 kASLR work (randomized >> >>> virtual address). >> >> So, I've done this and suddenly realized I hadn't boot-tested i386. It >> doesn't work, unfortunately. (Which I find strange: I'd expect 0day to >> have noticed...) >> >> Baoquan, have you tested this on 32-bit systems? I get a variety of >> failures. Either it boots okay, it reboots, or I get tons of pte >> errors like this: > > Hi Kees, > > I am sorry I didn't notice the change impacts i386. I got a i386 machine > and had tests. Found i386 can't take separate randomzation since there's > difference between i386 and x86_64. > > x86_64 has phys_base and can translate virt addr and phys addr according > to below formula: > > paddr = vaddr - __START_KERNEL_map + phys_base; > > However i386 can only do like this: > > paddr = vaddr - PAGE_OFFSET; > > Besides i386 has to reserve 128M for VMALLOC at the end of kernel > virtual address. So for i386 area 768M is the upper limit for > randomization. But I am fine with the KERNEL_IMAGE_SIZE, the old default > value. What do you say about this? > > So the plan should be keeping the old style of randomization for i386 > system: > > 1) Disable virtual address randomization in i386 case because it's > useless. This should be done in patch: > x86, KASLR: Randomize virtual address separately > > 2) Add an upper limit for physical randomization if it's i386 system. > x86, KASLR: Add physical address randomization >4G > > I just got a test machine in office, and haven't had time to change > code. You can change it directly, or I will do it tomorrow. I was thinking about the physical vs virtual on i386 as I woke up today. :) Thanks for confirming! These changes appear to make the series boot reliably on i386 (pardon any gmail-induced whitespace damage...): diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c index 5bae54b50d4c..3a58fe8acb8e 100644 --- a/arch/x86/boot/compressed/aslr.c +++ b/arch/x86/boot/compressed/aslr.c @@ -347,6 +347,10 @@ static void process_e820_entry(struct e820entry *entry, if (entry->type != E820_RAM) return; + /* On 32-bit, ignore entries entirely above our maximum. */ + if (IS_ENABLED(CONFIG_X86_32) && entry->addr >= KERNEL_IMAGE_SIZE) + return; + /* Ignore entries entirely below our minimum. */ if (entry->addr + entry->size < minimum) return; @@ -372,6 +376,11 @@ static void process_e820_entry(struct e820entry *entry, /* Reduce size by any delta from the original address. */ region.size -= region.start - start_orig; + /* On 32-bit, reduce region size to fit within max size. */ + if (IS_ENABLED(CONFIG_X86_32) && + region.start + region.size > KERNEL_IMAGE_SIZE) + region.size = KERNEL_IMAGE_SIZE - region.start; + /* Return if region can't contain decompressed kernel */ if (region.size < image_size) return; @@ -488,6 +497,8 @@ void choose_kernel_location(unsigned char *input, } /* Pick random virtual address starting from LOAD_PHYSICAL_ADDR. */ - random = find_random_virt_offset(LOAD_PHYSICAL_ADDR, output_size); + if (IS_ENABLED(CONFIG_X86_64)) + random = find_random_virt_offset(LOAD_PHYSICAL_ADDR, + output_size); *virt_offset = (unsigned char *)random; } I will split these chunks up into the correct patches and resend the series. If you get a chance, can you double-check this? Thanks! -Kees > > Thanks > >> >> [ 0.000000] clearing pte for ram above max_low_pfn: pfn: 37dcc pmd: >> f9144f7c pmd phys: 39144f7c pte: f9a1b730 pte phys: 39a1b730 >> >> Can you confirm? I suspect relocation problems, but ran out of time >> today to debug it. >> >> I have the entire series with cleaned up changelogs and various other >> refactorings up here now: >> >> http://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/log/?h=kaslr/highmem >> >> Thanks! >> >> -Kees >> >> -- >> Kees Cook >> Chrome OS & Brillo Security -- Kees Cook Chrome OS & Brillo Security
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.