[PATCH v5] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_* and EFI_LOADER_* from KASLR's choice

[Naoya Horiguchi]

On Mon, Aug 28, 2017 at 02:59:51PM +0800, Baoquan He wrote:
> Hi Naoya,
> 
> Thanks for this fix. I saw NEC had reported a bug to rhel previously,
> and the bug truly will corrupt OS, it can be fixed by this patch.
> 
> This patch looks good to me, just a small concern, please see below
> inline comment.
> 
> On 08/24/17 at 07:33pm, Naoya Horiguchi wrote:
> > KASLR chooses kernel location from E820_TYPE_RAM regions by walking over
> > e820 entries now. E820_TYPE_RAM includes EFI_BOOT_SERVICES_CODE and
> > EFI_BOOT_SERVICES_DATA, so those regions can be the target. According to
> > UEFI spec, all memory regions marked as EfiBootServicesCode and
> > EfiBootServicesData are available for free memory after the first call
> > of ExitBootServices(). So such regions should be usable for kernel on
> > spec basis.
> > 
> > In x86, however, we have some workaround for broken firmware, where we
> > keep such regions reserved until SetVirtualAddressMap() is done.
> > See the following code in should_map_region():
> > 
> > 	static bool should_map_region(efi_memory_desc_t *md)
> > 	{
> > 		...
> > 		/*
> > 		 * Map boot services regions as a workaround for buggy
> > 		 * firmware that accesses them even when they shouldn't.
> > 		 *
> > 		 * See efi_{reserve,free}_boot_services().
> > 		 */
> > 		if (md->type == EFI_BOOT_SERVICES_CODE ||
> > 			md->type == EFI_BOOT_SERVICES_DATA)
> > 				return false;
> > 
> > This workaround suppressed a boot crash, but potential issues still
> > remain because no one prevents the regions from overlapping with kernel
> > image by KASLR.
> > 
> > So let's make sure that EFI_BOOT_SERVICES_{CODE|DATA} regions are never
> > chosen as kernel memory for the workaround to work fine. Furthermore,
> > we choose kernel address only from EFI_CONVENTIONAL_MEMORY because it's
> > the only memory type we know to be free.
> 
> Here, I think it's better to present why EFI_CONVENTIONAL_MEMORY is the
> only memory type we should choose. EFI_BOOT_SERVICES_xxx has been clear
> to us why it's not good. It might be worth to saying something about
> EFI_LOADER_xxx why it's not ok to choose. Maybe one sentence to mention
> it and take pgd as exampel as Matt ever said.

According to EFI spec, EFI_LOADER_* regions can be used for some purposes
after ExitBootServices(), which is the reason for us to exclude them.

  Table 26. Memory Type Usage after ExitBootServices()

  ...
  EfiLoaderCode               The UEFI OS Loader and/or OS may use this memory as they see fit.
                              Note: the UEFI OS loader that called
                              EFI_BOOT_SERVICES.ExitBootServices() is utilizing one or
                              more EfiLoaderCode ranges.
  EfiLoaderData               The Loader and/or OS may use this memory as they see fit. Note: the
                              OS loader that called ExitBootServices() is utilizing one or more
                              EfiLoaderData ranges.
  ...
  EfiBootServicesCode         Memory available for general use.
  EfiBootServicesData         Memory available for general use.
  EfiConventionalMemory       Memory available for general use.

Here is an updated patch.

Thanks,
Naoya Horiguchi
---