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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20170728083329.GL24304@x1>
Date:   Fri, 28 Jul 2017 16:33:29 +0800
From:   Baoquan He <bhe@...hat.com>
To:     Ingo Molnar <mingo@...nel.org>
Cc:     linux-kernel@...r.kernel.org, keescook@...omium.org,
        matt@...eblueprint.co.uk, tglx@...utronix.de, hpa@...or.com,
        izumi.taku@...fujitsu.com, fanc.fnst@...fujitsu.com,
        thgarnie@...gle.com, n-horiguchi@...jp.nec.com, dyoung@...hat.com
Subject: Re: [PATCH v7] x86/boot/KASLR: Restrict kernel to be randomized in
 mirror regions

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