[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGXu5jLEDekACCizQaDinLZ63oc=40VpXfwBP5thagA3pCOOOA@mail.gmail.com>
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.
