| 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: <20171113110243.GH10474@x1>
Date: Mon, 13 Nov 2017 19:02:43 +0800
From: Baoquan He <bhe@...hat.com>
To: Chao Fan <fanc.fnst@...fujitsu.com>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org, hpa@...or.com,
tglx@...utronix.de, mingo@...hat.com, keescook@...omium.org,
yasu.isimatu@...il.com, indou.takao@...fujitsu.com,
caoj.fnst@...fujitsu.com, douly.fnst@...fujitsu.com
Subject: Re: [PATCH v2 2/4] kaslr: select the memory region in immovable node
to process
On 11/13/17 at 05:50pm, Chao Fan wrote:
> On Mon, Nov 13, 2017 at 05:26:24PM +0800, Baoquan He wrote:
> >> >> +static bool select_immovable_node(struct mem_vector region,
> >> >> + unsigned long long minimum,
> >> >> + unsigned long long image_size)
> >> >> +{
> >> >
> >> >About this patch, I just want to notice two things:
> >> >1) From the current code, 'movable_node' kernel parameter is exclusive.
> >> >In find_zone_movable_pfns_for_nodes(), you can see that 'kernelcore' and
> >> >'movablecore' will be ignored as long as 'movable_node' is specified.
> >> >Please also consider this in your code here. If 'movable_node' has to be
> >> >specified too, and need skip the kernel mirror handling.
> >> >
> >>
> >> Thanks for the notice, I will add the similar operation.
> >
> >No, I meant if movable_node is specified, we have to ignore
> >'kernelcore=', then we may need skip the kernel mirror handling. Since
> >kernel mirror is enabled by 'kernelcore=mirror'.
> >
>
> Yes, the two parameters are conflict.
> Then if "movable_node" specified, we shoul not consider mirror region.
> I think there can be one another patch to handle this, but not in this
> serials. I will add the change.
Well, no.
You didn't get what I said. In process_efi_entries() mirrorred
regions are picked. If movable_node specified, mirror feature will be
ignored. That has been handled in find_zone_movable_pfns_for_nodes()
very well. So you need make it be consistent.
And if you add a new kernel para, like immovable_mem=xx@xx, movable_node
will be expected. Anyway, please read find_zone_movable_pfns_for_nodes()
carefully.
Thanks
Baoquan
>
> >
> >>
> >> >2)process_mem_region() is a key function to process the available memory
> >> >regions. Please don't make another process_mem_region() like below. You
> >> >can write a small helper function to find the immovable_mem[] which is
> >> >intersecting with the passed in memory region and use clamp() to get
> >> >the real available region. You 'REALLY' don't need to split the region
> >> >in your so called 'select_immovable_node()' function here.
> >> >
> >>
> >> OK, I will try to make a new method, which is smaller and better to
> >> filter the regions.
> >>
> >> Thanks,
> >> Chao Fan
> >>
> >> >PLEASE elaborate more on these details before post.
> >> >
> >> >Thanks
> >> >Baoquan
> >> >
> >> >> + int i;
> >> >> +
> >> >> + /* If no immovable_mem stored, use region directly */
> >> >> + if (num_immovable_region == 0) {
> >> >> + process_mem_region(®ion, minimum, image_size);
> >> >> +
> >> >> + if (slot_area_index == MAX_SLOT_AREA) {
> >> >> + debug_putstr("Aborted memmap scan (slot_areas full)!\n");
> >> >> + return 1;
> >> >> + }
> >> >> + } else {
> >> >> + /*
> >> >> + * Walk all immovable regions, and filter the intersection
> >> >> + * to process_mem_region.
> >> >> + */
> >> >> + for (i = 0; i < num_immovable_region; i++) {
> >> >> + struct mem_vector entry;
> >> >> + unsigned long long start, end, select_end, region_end;
> >> >> +
> >> >> + region_end = region.start + region.size - 1;
> >> >> + start = immovable_mem[i].start;
> >> >> + end = start + immovable_mem[i].size - 1;
> >> >> +
> >> >> + if (region_end < start || region.start > end)
> >> >> + continue;
> >> >> +
> >> >> + /* May split one region to several entries. */
> >> >> + entry.start = start > region.start ?
> >> >> + start : region.start;
> >> >> + select_end = end > region_end ? region_end : end;
> >> >> +
> >> >> + entry.size = select_end - entry.start + 1;
> >> >> +
> >> >> + process_mem_region(&entry, minimum, image_size);
> >> >> +
> >> >> + if (slot_area_index == MAX_SLOT_AREA) {
> >> >> + debug_putstr("Aborted memmap scan (slot_areas full)!\n");
> >> >> + return 1;
> >> >> + }
> >> >> + }
> >> >> + }
> >> >> + return 0;
> >> >> +}
> >> >> +
> >> >> #ifdef CONFIG_EFI
> >> >> /*
> >> >> * Returns true if mirror region found (and must have been processed
> >> >> @@ -699,11 +747,9 @@ process_efi_entries(unsigned long minimum, unsigned long image_size)
> >> >>
> >> >> region.start = md->phys_addr;
> >> >> region.size = md->num_pages << EFI_PAGE_SHIFT;
> >> >> - process_mem_region(®ion, minimum, image_size);
> >> >> - if (slot_area_index == MAX_SLOT_AREA) {
> >> >> - debug_putstr("Aborted EFI scan (slot_areas full)!\n");
> >> >> +
> >> >> + if (select_immovable_node(region, minimum, image_size))
> >> >> break;
> >> >> - }
> >> >> }
> >> >> return true;
> >> >> }
> >> >> @@ -730,11 +776,9 @@ static void process_e820_entries(unsigned long minimum,
> >> >> continue;
> >> >> region.start = entry->addr;
> >> >> region.size = entry->size;
> >> >> - process_mem_region(®ion, minimum, image_size);
> >> >> - if (slot_area_index == MAX_SLOT_AREA) {
> >> >> - debug_putstr("Aborted e820 scan (slot_areas full)!\n");
> >> >> +
> >> >> + if (select_immovable_node(region, minimum, image_size))
> >> >> break;
> >> >> - }
> >> >> }
> >> >> }
> >> >>
> >> >> --
> >> >> 2.13.6
> >> >>
> >> >>
> >> >>
> >> >
> >> >
> >>
> >>
> >
> >
>
>
Powered by blists - more mailing lists