Re: [PATCH] x86/boot/KASLR: exclude EFI_BOOT_SERVICES_{CODE|DATA} from KASLR's choice

[Matt Fleming]

On Fri, 07 Jul, at 11:07:59AM, 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.

What about the GDT that we allocate in the x86 EFI boot stub as
EFI_LOADER_DATA? Are there functions to avoid that too?

What about any future uses we add? Who's going to remember to patch
the kaslr code which now duplicates some of the EFI memory map logic?

All of these problems can avoided if you just stick with
EFI_CONVENTIONAL_MEMORY.

Honestly, how much memory do we expect to waste if we ignore
EFI_LOADER_* regions?

Also, the fact that you're referencing EFI-specific boot quirks in the
kaslr code should be a massive red flag that you're playing with the
innards of the EFI subsystem.