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

[Baoquan He]

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