lkml.org 
[lkml]   [2017]   [Jul]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice
    Date
    On Fri, Jul 07, 2017 at 11:58:14AM +0100, Matt Fleming wrote:
    > On Fri, 07 Jul, at 06:11:24AM, Naoya Horiguchi wrote:
    > > On Fri, Jul 07, 2017 at 11:07:59AM +0800, 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.
    > >
    > > Hi Matt, Baoquan,
    > >
    > > I added these three checks to accept any regions corresponding to
    > > E820_TYPE_RAM except EFI_BOOT_SERVICES_*, just thinking of that it's minimum
    > > surprising. Baoquan gave a good justification on that, so I'll leave it
    > > as-is in next version.
    >
    > I disagree. The least surprising option would be to use the region
    > type that everyone (boot loader, kernel, firmware) agrees is free:
    > EFI_CONVENTIONAL_MEMORY.

    OK, I will do it.

    \
     
     \ /
      Last update: 2017-07-10 07:48    [W:2.965 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site