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
| ||
|
Message-ID: <20171207010935.GC28884@localhost.localdomain> Date: Thu, 7 Dec 2017 09:09:36 +0800 From: Chao Fan <fanc.fnst@...fujitsu.com> To: Kees Cook <keescook@...omium.org> CC: Baoquan He <bhe@...hat.com>, LKML <linux-kernel@...r.kernel.org>, X86 ML <x86@...nel.org>, "H. Peter Anvin" <hpa@...or.com>, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, <yasu.isimatu@...il.com>, <indou.takao@...fujitsu.com>, <caoj.fnst@...fujitsu.com>, Dou Liyang <douly.fnst@...fujitsu.com> Subject: Re: [PATCH v3 2/4] kaslr: calculate the memory region in immovable node On Wed, Dec 06, 2017 at 04:11:04PM -0800, Kees Cook wrote: >On Wed, Dec 6, 2017 at 2:02 AM, Chao Fan <fanc.fnst@...fujitsu.com> wrote: >> On Wed, Dec 06, 2017 at 05:28:00PM +0800, Baoquan He wrote: >>>On 12/05/17 at 11:40am, Kees Cook wrote: >>>> On Tue, Dec 5, 2017 at 12:51 AM, Chao Fan <fanc.fnst@...fujitsu.com> wrote: >>>> > If there is no immovable memory region specified, go on the old code. >>>> > There are several conditons: >>>> > 1. CONFIG_MEMORY_HOTPLUG is not specified to y. >>>> > 2. immovable_mem= is not specified. >>>> > >>>> > Otherwise, calculate the intersecting between memmap entry and >>>> > immovable memory. >>>> >>>> Instead of copy/pasting code between process_efi_entries() and >>>> process_e820_entries(), I'd rather that process_mem_region() is >>>> modified to deal with immovable regions. >>> >>>If put it into process_mem_region(), one level of loop is added. How >> >> Yes, one new loop will add ahead of the while() in process_mem_region >> then the code may look like: >> >> @@ -509,6 +555,24 @@ static void process_mem_region(struct mem_vector *entry, >> region.start = cur_entry.start; >> region.size = cur_entry.size; >> >> +#ifdef CONFIG_MEMORY_HOTPLUG >> +next: >> + if (num_immovable_mem > 0) { >> + unsigned long long start, reg_end; >> + >> + if (!mem_overlaps(&entry, &immovable_mem[i])) >> + goto out; >> + >> + start = immovable_mem[i].start; >> + end = start + immovable_mem[i].size; >> + >> + region.start = clamp(cur_entry.start, start, end); >> + reg_end = clamp(cur_entry.start + cur_entry.size, start, end); >> + >> + region.size = region_end - region.start; >> + } >> +#endif >> + >> /* Give up if slot area array is full. */ >> while (slot_area_index < MAX_SLOT_AREA) { >> start_orig = region.start; >> @@ -522,7 +586,7 @@ static void process_mem_region(struct mem_vector *entry, >> >> /* Did we raise the address above the passed in memory entry? */ >> if (region.start > cur_entry.start + cur_entry.size) >> - return; >> + goto out; >> >> /* Reduce size by any delta from the original address. */ >> region.size -= region.start - start_orig; >> @@ -534,12 +598,12 @@ static void process_mem_region(struct mem_vector *entry, >> >> /* Return if region can't contain decompressed kernel */ >> if (region.size < image_size) >> - return; >> + goto out; >> >> /* If nothing overlaps, store the region and return. */ >> if (!mem_avoid_overlap(®ion, &overlap)) { >> store_slot_info(®ion, image_size); >> - return; >> + goto out; >> } >> >> /* Store beginning of region if holds at least image_size. */ >> @@ -553,12 +617,20 @@ static void process_mem_region(struct mem_vector *entry, >> >> /* Return if overlap extends to or past end of region. */ >> if (overlap.start + overlap.size >= region.start + region.size) >> - return; >> + goto out; >> >> /* Clip off the overlapping region and start over. */ >> region.size -= overlap.start - region.start + overlap.size; >> region.start = overlap.start + overlap.size; >> } >> + >> +out: >> +#ifdef CONFIG_MEMORY_HOTPLUG >> + i++; >> + if (i < num_immovable_mem) >> + goto next; >> +#endif >> + return; >> } >> >>>about changing it like below. If no immovable_mem, just process the >>>region in process_immovable_mem(). This we don't need to touch >>>process_mem_region(). >> >> Yes, Baoquan's method will make all change be in one function. >> Kees, how do you think, which is better? > >I prefer Baoquan's approach, though I don't like the function names. >:) Perhaps rename process_mem_region() to slots_count() (to match >slots_fetch_random()) and rename process_immovable_mem() to >process_mem_region(). Thanks for your review, I will change and send the new version. Thanks, Chao Fan > >-Kees > >-- >Kees Cook >Pixel Security > >
Powered by blists - more mailing lists