Re: [PATCH v7] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions

[Baoquan He <bhe@redhat.com>]

Hi Ingo,

On 07/28/17 at 08:46am, Ingo Molnar wrote:
> 
> > +	pmap =  e->efi_memmap;
> > +#else
> > +	pmap = (e->efi_memmap | ((__u64)e->efi_memmap_hi << 32));
> > +#endif
> > +
> > +	nr_desc = e->efi_memmap_size / e->efi_memdesc_size;
> > +	for (i = 0; i < nr_desc; i++) {
> > +		md = (efi_memory_desc_t *)(pmap + (i * e->efi_memdesc_size));
> > +		if (md->attribute & EFI_MEMORY_MORE_RELIABLE) {
> > +			region.start = md->phys_addr;
> > +			region.size = md->num_pages << EFI_PAGE_SHIFT;
> > +			process_mem_region(&region, minimum, image_size);
> > +			efi_mirror_found = true;
> > +
> > +			if (slot_area_index == MAX_SLOT_AREA) {
> > +				debug_putstr("Aborted EFI scan (slot_areas full)!\n");
> > +				break;
> > +			}
> > +		}
> > +	}
> 
> So I suggested this before: if you treat 'md' as an array of elements (which it 
> is), then the type cast can be moved to a more natural point, where we do address 
> calculations anyway:
> 
> 	md = (efi_memory_desc_t *)(e->efi_memmap | ((__u64)e->efi_memmap_hi << 32)));
> 
> The 'pmap' variable can be eliminated and all places in the loop that use 'md->' 
> can use 'md[i].'.

Thanks for reviewing this patch. Before when I read EFI code and found
almost all EFI codes take step of e->efi_memdesc_size long to forward to
get each EFI memmap, that's why I use it the same way here. I guess EFI
may not guarantee that each EFI memory map entry of 'efi_memory_desc_t'
occupy the whole space of e->efi_memdesc_size. In v6 post, you
questioned this, and Matt helped to confirm that EFI spec is suggesting
that way. Sorry, I trimmed the unrelated code in v6 reply, that could
make Matt's reply ignored.

Thanks
Baoquan