From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751663AbdG1Idh (ORCPT ); Fri, 28 Jul 2017 04:33:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52000 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751007AbdG1Idg (ORCPT ); Fri, 28 Jul 2017 04:33:36 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 50FAD52C59 Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=bhe@redhat.com Date: Fri, 28 Jul 2017 16:33:29 +0800 From: Baoquan He To: Ingo Molnar Cc: linux-kernel@vger.kernel.org, keescook@chromium.org, matt@codeblueprint.co.uk, tglx@linutronix.de, hpa@zytor.com, izumi.taku@jp.fujitsu.com, fanc.fnst@cn.fujitsu.com, thgarnie@google.com, n-horiguchi@ah.jp.nec.com, dyoung@redhat.com Subject: Re: [PATCH v7] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions Message-ID: <20170728083329.GL24304@x1> References: <1501041620-4965-1-git-send-email-bhe@redhat.com> <20170728064614.lcntuaovomcqwxx4@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170728064614.lcntuaovomcqwxx4@gmail.com> User-Agent: Mutt/1.7.0 (2016-08-17) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Fri, 28 Jul 2017 08:33:35 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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(®ion, 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