[RFC][PATCH 0/2] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if existed

[RFC][PATCH 0/2] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if existed

[Baoquan He]

Our customer reported that Kernel text may be located on non-mirror
region (movable zone) when both address range mirroring feature and
KASLR are enabled.

The functions of address range mirroring feature are as follows.
- The physical memory region whose descriptors in EFI memory map have
  EFI_MEMORY_MORE_RELIABLE attribute (bit: 16) are mirrored
- The function arranges such mirror region into normal zone and other region
  into movable zone in order to locate kernel code and data on mirror region

So we need restrict kernel to be located inside mirror region if it
is existed.

The method is very simple. If efi is enabled, just iterate all efi
memory map and pick up mirror region to process for adding candidate
of slot. If efi disabled or no mirror region existed, still process
e820 memory map. This won't bring much efficiency loss, at worst we
just go through all efi memory maps and found no mirror.

One question:
>From code, though mirror regions are existed, they are meaningful only
if kernelcore=mirror kernel option is specified. Not sure if my understanding
is correct.

NOTE:
I haven't got a machine with efi mirror region enabled, so only test the
e820 map processing case and the case of no mirror region on efi machine.
So set this as a RFC patchset, will post formal one after above question
is made clear and mirror issue test passed.

Baoquan He (2):
  x86/boot/KASLR: Adapt process_e820_entry for all kinds of memory map
  x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if
    existed

 arch/x86/boot/compressed/kaslr.c | 129 +++++++++++++++++++++++++++++++--------
 1 file changed, 104 insertions(+), 25 deletions(-)

-- 
2.5.5

[PATCH 1/2] x86/boot/KASLR: Adapt process_e820_entry for all kinds of memory map

[Baoquan He]

Now function process_e820_entry is only used to process e820 memory
entry. Adapt it for memory region only, not just for e820. Later
we will use it to process efi mirror regions.

So rename the original process_e820_entry to process_mem_region, and
extract and wrap the e820 specific processing code into process_e820_entry.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 arch/x86/boot/compressed/kaslr.c | 60 ++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 27 deletions(-)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index fe318b4..c2ed051 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -479,35 +479,31 @@ static unsigned long slots_fetch_random(void)
 	return 0;
 }
 
-static void process_e820_entry(struct boot_e820_entry *entry,
+static void process_mem_region(struct mem_vector *entry,
 			       unsigned long minimum,
 			       unsigned long image_size)
 {
 	struct mem_vector region, overlap;
 	struct slot_area slot_area;
 	unsigned long start_orig, end;
-	struct boot_e820_entry cur_entry;
-
-	/* Skip non-RAM entries. */
-	if (entry->type != E820_TYPE_RAM)
-		return;
+	struct mem_vector cur_entry;
 
 	/* On 32-bit, ignore entries entirely above our maximum. */
-	if (IS_ENABLED(CONFIG_X86_32) && entry->addr >= KERNEL_IMAGE_SIZE)
+	if (IS_ENABLED(CONFIG_X86_32) && entry->start >= KERNEL_IMAGE_SIZE)
 		return;
 
 	/* Ignore entries entirely below our minimum. */
-	if (entry->addr + entry->size < minimum)
+	if (entry->start + entry->size < minimum)
 		return;
 
 	/* Ignore entries above memory limit */
-	end = min(entry->size + entry->addr, mem_limit);
-	if (entry->addr >= end)
+	end = min(entry->size + entry->start, mem_limit);
+	if (entry->start >= end)
 		return;
-	cur_entry.addr = entry->addr;
-	cur_entry.size = end - entry->addr;
+	cur_entry.start = entry->start;
+	cur_entry.size = end - entry->start;
 
-	region.start = cur_entry.addr;
+	region.start = cur_entry.start;
 	region.size = cur_entry.size;
 
 	/* Give up if slot area array is full. */
@@ -522,7 +518,7 @@ static void process_e820_entry(struct boot_e820_entry *entry,
 		region.start = ALIGN(region.start, CONFIG_PHYSICAL_ALIGN);
 
 		/* Did we raise the address above this e820 region? */
-		if (region.start > cur_entry.addr + cur_entry.size)
+		if (region.start > cur_entry.start + cur_entry.size)
 			return;
 
 		/* Reduce size by any delta from the original address. */
@@ -562,12 +558,31 @@ static void process_e820_entry(struct boot_e820_entry *entry,
 	}
 }
 
-static unsigned long find_random_phys_addr(unsigned long minimum,
-					   unsigned long image_size)
+static void process_e820_entry(unsigned long minimum,unsigned long image_size)
 {
 	int i;
-	unsigned long addr;
+	struct mem_vector region;
+	struct boot_e820_entry *entry;
+
+	/* Verify potential e820 positions, appending to slots list. */
+        for (i = 0; i < boot_params->e820_entries; i++) {
+                entry = &boot_params->e820_table[i];
+                /* Skip non-RAM entries. */
+                if (entry->type != E820_TYPE_RAM)
+                        continue;
+                region.start = entry->addr;
+                region.size = entry->size;
+                process_mem_region(&region, minimum, image_size);
+                if (slot_area_index == MAX_SLOT_AREA) {
+                        debug_putstr("Aborted e820 scan (slot_areas full)!\n");
+                        break;
+                }
+        }
+}
 
+static unsigned long find_random_phys_addr(unsigned long minimum,
+					   unsigned long image_size)
+{
 	/* Check if we had too many memmaps. */
 	if (memmap_too_large) {
 		debug_putstr("Aborted e820 scan (more than 4 memmap= args)!\n");
@@ -577,16 +592,7 @@ static unsigned long find_random_phys_addr(unsigned long minimum,
 	/* Make sure minimum is aligned. */
 	minimum = ALIGN(minimum, CONFIG_PHYSICAL_ALIGN);
 
-	/* Verify potential e820 positions, appending to slots list. */
-	for (i = 0; i < boot_params->e820_entries; i++) {
-		process_e820_entry(&boot_params->e820_table[i], minimum,
-				   image_size);
-		if (slot_area_index == MAX_SLOT_AREA) {
-			debug_putstr("Aborted e820 scan (slot_areas full)!\n");
-			break;
-		}
-	}
-
+	process_e820_entry(minimum, image_size);
 	return slots_fetch_random();
 }
 
-- 
2.5.5

[PATCH 2/2] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if existed

[Baoquan He]

Kernel text may be located on non-mirror region (movable zone) when both
address range mirroring feature and KASLR are enabled.

The functions of address range mirroring feature arranges such mirror
region into normal zone and other region into movable zone in order to
locate kernel code and data on mirror region. The physical memory region
whose descriptors in EFI memory map have EFI_MEMORY_MORE_RELIABLE attribute
(bit: 16) are mirrored.

If efi is detected, iterate efi memory map and pick up the mirror region
to process for adding candidate of randomization slot. If efi is disabled
or no mirror region found, still process e820 memory map.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 arch/x86/boot/compressed/kaslr.c | 73 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index c2ed051..a6aa69e 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -37,7 +37,9 @@
 #include <linux/uts.h>
 #include <linux/utsname.h>
 #include <linux/ctype.h>
+#include <linux/efi.h>
 #include <generated/utsrelease.h>
+#include <asm/efi.h>
 
 /* Macros used by the included decompressor code below. */
 #define STATIC
@@ -558,6 +560,73 @@ static void process_mem_region(struct mem_vector *entry,
 	}
 }
 
+/* This variable marks if efi mirror regions have been handled. */
+bool efi_mirror_found = false;
+
+static void process_efi_entry(unsigned long minimum, unsigned long image_size)
+{
+	struct efi_info *e = &boot_params->efi_info;
+	efi_memory_desc_t *md;
+	struct mem_vector region;
+	unsigned long pmap;
+	bool is_efi = false;
+	u32 nr_desc;
+	int i;
+	unsigned long addr;
+	u64 end;
+	char *cmdline = (char *)get_cmd_line_ptr();
+	char *str;
+	char *signature;
+
+
+#ifdef CONFIG_EFI
+	signature = (char *)&boot_params->efi_info.efi_loader_signature;
+#endif
+	if (strncmp(signature, EFI32_LOADER_SIGNATURE, 4) &&
+	    strncmp(signature, EFI64_LOADER_SIGNATURE, 4))
+		return;
+
+	/*
+	 * Mirrored regions are meaningful only if "kernelcore=mirror"
+	 * specified.
+	 */
+	str = strstr(cmdline, "kernelcore=");
+	if (!str)
+		return;
+	str += strlen("kernelcore=");
+	if (strncmp(str, "mirror", 6))
+		return;
+
+#ifdef CONFIG_X86_32
+       /* Can't handle data above 4GB at this time */
+       if (e->efi_memmap_hi) {
+                warn("Memory map is above 4GB, disabling EFI.\n");
+                return -EINVAL;
+        }
+        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;
+		}
+		debug_putaddr(i);
+		debug_putaddr(md->attribute);
+		debug_putaddr(md->phys_addr);
+		end = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - 1;
+		debug_putaddr(end);
+       }
+
+       return;
+}
+
 static void process_e820_entry(unsigned long minimum,unsigned long image_size)
 {
 	int i;
@@ -592,6 +661,10 @@ static unsigned long find_random_phys_addr(unsigned long minimum,
 	/* Make sure minimum is aligned. */
 	minimum = ALIGN(minimum, CONFIG_PHYSICAL_ALIGN);
 
+	process_efi_entry(minimum, image_size);
+	if (efi_mirror_found)
+		return slots_fetch_random();
+
 	process_e820_entry(minimum, image_size);
 	return slots_fetch_random();
 }
-- 
2.5.5

Re: [RFC][PATCH 0/2] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if existed

[Baoquan He]

Sorry, forget adding Taku to the list.

Hi Taku,

On 06/15/17 at 03:52pm, Baoquan He wrote:
> Our customer reported that Kernel text may be located on non-mirror
> region (movable zone) when both address range mirroring feature and
> KASLR are enabled.
> 
> The functions of address range mirroring feature are as follows.
> - The physical memory region whose descriptors in EFI memory map have
>   EFI_MEMORY_MORE_RELIABLE attribute (bit: 16) are mirrored
> - The function arranges such mirror region into normal zone and other region
>   into movable zone in order to locate kernel code and data on mirror region
> 
> So we need restrict kernel to be located inside mirror region if it
> is existed.
> 
> The method is very simple. If efi is enabled, just iterate all efi
> memory map and pick up mirror region to process for adding candidate
> of slot. If efi disabled or no mirror region existed, still process
> e820 memory map. This won't bring much efficiency loss, at worst we
> just go through all efi memory maps and found no mirror.
> 
> One question:
> From code, though mirror regions are existed, they are meaningful only
> if kernelcore=mirror kernel option is specified. Not sure if my understanding
> is correct.

Since you are the author of kernelcore=mirror related code and expert on
mirror feature, could you help answer above question?

Thanks
Baoquan
> 
> NOTE:
> I haven't got a machine with efi mirror region enabled, so only test the
> e820 map processing case and the case of no mirror region on efi machine.
> So set this as a RFC patchset, will post formal one after above question
> is made clear and mirror issue test passed.
> 
> Baoquan He (2):
>   x86/boot/KASLR: Adapt process_e820_entry for all kinds of memory map
>   x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if
>     existed
> 
>  arch/x86/boot/compressed/kaslr.c | 129 +++++++++++++++++++++++++++++++--------
>  1 file changed, 104 insertions(+), 25 deletions(-)
> 
> -- 
> 2.5.5
> 

RE: [RFC][PATCH 0/2] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if existed

[Izumi, Taku]

Dear Baoquan,

> > Our customer reported that Kernel text may be located on non-mirror
> > region (movable zone) when both address range mirroring feature and
> > KASLR are enabled.

   I know your customer :)

> > The functions of address range mirroring feature are as follows.
> > - The physical memory region whose descriptors in EFI memory map have
> >   EFI_MEMORY_MORE_RELIABLE attribute (bit: 16) are mirrored
> > - The function arranges such mirror region into normal zone and other
> region
> >   into movable zone in order to locate kernel code and data on mirror
> > region
> >
> > So we need restrict kernel to be located inside mirror region if it is
> > existed.
> >
> > The method is very simple. If efi is enabled, just iterate all efi
> > memory map and pick up mirror region to process for adding candidate
> > of slot. If efi disabled or no mirror region existed, still process
> > e820 memory map. This won't bring much efficiency loss, at worst we
> > just go through all efi memory maps and found no mirror.
> >
> > One question:
> > From code, though mirror regions are existed, they are meaningful only
> > if kernelcore=mirror kernel option is specified. Not sure if my
> > understanding is correct.

   Your understanding is almost correct. 
   Only when "kernelcore=mirror" specified, the above procedure works.
   But, if mirrored regions are existed, bootmem allocator tries to 
   allocate from mirrored region independently of "kerenelcore=mirror" option.

   So, IMHO, kernel text is important, so putting it to mirrored (more reliable)
   region is reasonable whether or not "kernelcore=mirror" is specified.

   Anyway thanks for submitting patch.
   We have Address Range Mirroring capable machine, so we'll test your patch.

  Sincerely,
  Taku Izumi

> 
> Since you are the author of kernelcore=mirror related code and expert on
> mirror feature, could you help answer above question?
> 
> Thanks
> Baoquan
> >
> > NOTE:
> > I haven't got a machine with efi mirror region enabled, so only test
> > the
> > e820 map processing case and the case of no mirror region on efi machine.
> > So set this as a RFC patchset, will post formal one after above
> > question is made clear and mirror issue test passed.
> >
> > Baoquan He (2):
> >   x86/boot/KASLR: Adapt process_e820_entry for all kinds of memory map
> >   x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if
> >     existed
> >
> >  arch/x86/boot/compressed/kaslr.c | 129
> > +++++++++++++++++++++++++++++++--------
> >  1 file changed, 104 insertions(+), 25 deletions(-)
> >
> > --
> > 2.5.5
> >

Re: [RFC][PATCH 0/2] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if existed

['Baoquan He']

On 06/15/17 at 08:34am, Izumi, Taku wrote:
> Dear Baoquan,
> 
> > > Our customer reported that Kernel text may be located on non-mirror
> > > region (movable zone) when both address range mirroring feature and
> > > KASLR are enabled.
> 
>    I know your customer :)

LOL, have to agree.

> > > The method is very simple. If efi is enabled, just iterate all efi
> > > memory map and pick up mirror region to process for adding candidate
> > > of slot. If efi disabled or no mirror region existed, still process
> > > e820 memory map. This won't bring much efficiency loss, at worst we
> > > just go through all efi memory maps and found no mirror.
> > >
> > > One question:
> > > From code, though mirror regions are existed, they are meaningful only
> > > if kernelcore=mirror kernel option is specified. Not sure if my
> > > understanding is correct.
> 
>    Your understanding is almost correct. 
>    Only when "kernelcore=mirror" specified, the above procedure works.
>    But, if mirrored regions are existed, bootmem allocator tries to 
>    allocate from mirrored region independently of "kerenelcore=mirror" option.
> 
>    So, IMHO, kernel text is important, so putting it to mirrored (more reliable)
>    region is reasonable whether or not "kernelcore=mirror" is specified.

Ah, yeah, thanks for telling. So at boot time memblock will put mirror
region in highest priority to allocate. Then let me remove the
kernelcore=mirror handling code, the process_efi_entry will be simpler.

commit a3f5bafcc04aaf62990e0cf3ced1cc6d8dc6fe95
Author: Tony Luck <tony.luck@intel.com>
Date:   Wed Jun 24 16:58:12 2015 -0700

    mm/memblock: allocate boot time data structures from mirrored memory

> 
>    Anyway thanks for submitting patch.
>    We have Address Range Mirroring capable machine, so we'll test your patch.

Thanks a lot for help, Yasuaki Ishimatsu said he also will loan me a testing
machine when it's available.

Thanks
Baoquan

> > 
> > >
> > > NOTE:
> > > I haven't got a machine with efi mirror region enabled, so only test
> > > the
> > > e820 map processing case and the case of no mirror region on efi machine.
> > > So set this as a RFC patchset, will post formal one after above
> > > question is made clear and mirror issue test passed.
> > >
> > > Baoquan He (2):
> > >   x86/boot/KASLR: Adapt process_e820_entry for all kinds of memory map
> > >   x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if
> > >     existed
> > >
> > >  arch/x86/boot/compressed/kaslr.c | 129
> > > +++++++++++++++++++++++++++++++--------
> > >  1 file changed, 104 insertions(+), 25 deletions(-)
> > >
> > > --
> > > 2.5.5
> > >
> 

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2064 bytes --]

Hi Baoquan,

[auto build test WARNING on tip/auto-latest]
[also build test WARNING on next-20170615]
[cannot apply to tip/x86/core v4.12-rc5]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Baoquan-He/x86-boot-KASLR-Restrict-kernel-to-be-randomized-in-mirror-regions-if-existed/20170615-204125
config: i386-defconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   arch/x86/boot/compressed/kaslr.c: In function 'process_efi_entry':
>> arch/x86/boot/compressed/kaslr.c:604:24: warning: 'return' with a value, in function returning void
                    return -EINVAL;
                           ^
   arch/x86/boot/compressed/kaslr.c:566:13: note: declared here
    static void process_efi_entry(unsigned long minimum, unsigned long image_size)
                ^~~~~~~~~~~~~~~~~

vim +/return +604 arch/x86/boot/compressed/kaslr.c

   588	
   589		/*
   590		 * Mirrored regions are meaningful only if "kernelcore=mirror"
   591		 * specified.
   592		 */
   593		str = strstr(cmdline, "kernelcore=");
   594		if (!str)
   595			return;
   596		str += strlen("kernelcore=");
   597		if (strncmp(str, "mirror", 6))
   598			return;
   599	
   600	#ifdef CONFIG_X86_32
   601	       /* Can't handle data above 4GB at this time */
   602	       if (e->efi_memmap_hi) {
   603	                warn("Memory map is above 4GB, disabling EFI.\n");
 > 604	                return -EINVAL;
   605	        }
   606	        pmap =  e->efi_memmap;
   607	#else
   608	        pmap = (e->efi_memmap | ((__u64)e->efi_memmap_hi << 32));
   609	#endif
   610	
   611		nr_desc = e->efi_memmap_size / e->efi_memdesc_size;
   612		for (i = 0; i < nr_desc; i++) {

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26165 bytes --]

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

[Baoquan He]

On 06/15/17 at 10:04pm, kbuild test robot wrote:
> Hi Baoquan,
> 
> [auto build test WARNING on tip/auto-latest]
> [also build test WARNING on next-20170615]
> [cannot apply to tip/x86/core v4.12-rc5]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Baoquan-He/x86-boot-KASLR-Restrict-kernel-to-be-randomized-in-mirror-regions-if-existed/20170615-204125
> config: i386-defconfig (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> All warnings (new ones prefixed by >>):
> 
>    arch/x86/boot/compressed/kaslr.c: In function 'process_efi_entry':
> >> arch/x86/boot/compressed/kaslr.c:604:24: warning: 'return' with a value, in function returning void
>                     return -EINVAL;
>                            ^
>    arch/x86/boot/compressed/kaslr.c:566:13: note: declared here
>     static void process_efi_entry(unsigned long minimum, unsigned long image_size)
>                 ^~~~~~~~~~~~~~~~~
> 
> vim +/return +604 arch/x86/boot/compressed/kaslr.c
> 
>    588	
>    589		/*
>    590		 * Mirrored regions are meaningful only if "kernelcore=mirror"
>    591		 * specified.
>    592		 */
>    593		str = strstr(cmdline, "kernelcore=");
>    594		if (!str)
>    595			return;
>    596		str += strlen("kernelcore=");
>    597		if (strncmp(str, "mirror", 6))
>    598			return;
>    599	
>    600	#ifdef CONFIG_X86_32
>    601	       /* Can't handle data above 4GB at this time */
>    602	       if (e->efi_memmap_hi) {
>    603	                warn("Memory map is above 4GB, disabling EFI.\n");
>  > 604	                return -EINVAL;

Yeah, this need be changed. This patchset is not a formal one, there was
unclear issue. Now it has been clarified, will repost a formal one after
test on efi mirror machine passed.


>    605	        }
>    606	        pmap =  e->efi_memmap;
>    607	#else
>    608	        pmap = (e->efi_memmap | ((__u64)e->efi_memmap_hi << 32));
>    609	#endif
>    610	
>    611		nr_desc = e->efi_memmap_size / e->efi_memdesc_size;
>    612		for (i = 0; i < nr_desc; i++) {
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH 1/2] x86/boot/KASLR: Adapt process_e820_entry for all kinds of memory map

[Chao Fan]

On Thu, Jun 15, 2017 at 03:52:48PM +0800, Baoquan He wrote:
>Now function process_e820_entry is only used to process e820 memory
>entry. Adapt it for memory region only, not just for e820. Later
>we will use it to process efi mirror regions.
>
>So rename the original process_e820_entry to process_mem_region, and
>extract and wrap the e820 specific processing code into process_e820_entry.
>
>Signed-off-by: Baoquan He <bhe@redhat.com>

Hi Bao,

Maybe some code annotations should be changed, for example:

	/* Walk e820 and find a random address. */
	random_addr = find_random_phys_addr(min_addr, output_size);

Cause you will walk efi map firstly, and then e820 map(or not).

Thanks,
Chao Fan

>---
> arch/x86/boot/compressed/kaslr.c | 60 ++++++++++++++++++++++------------------
> 1 file changed, 33 insertions(+), 27 deletions(-)
>
>diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
>index fe318b4..c2ed051 100644
>--- a/arch/x86/boot/compressed/kaslr.c
>+++ b/arch/x86/boot/compressed/kaslr.c
>@@ -479,35 +479,31 @@ static unsigned long slots_fetch_random(void)
> 	return 0;
> }
> 
>-static void process_e820_entry(struct boot_e820_entry *entry,
>+static void process_mem_region(struct mem_vector *entry,
> 			       unsigned long minimum,
> 			       unsigned long image_size)
> {
> 	struct mem_vector region, overlap;
> 	struct slot_area slot_area;
> 	unsigned long start_orig, end;
>-	struct boot_e820_entry cur_entry;
>-
>-	/* Skip non-RAM entries. */
>-	if (entry->type != E820_TYPE_RAM)
>-		return;
>+	struct mem_vector cur_entry;
> 
> 	/* On 32-bit, ignore entries entirely above our maximum. */
>-	if (IS_ENABLED(CONFIG_X86_32) && entry->addr >= KERNEL_IMAGE_SIZE)
>+	if (IS_ENABLED(CONFIG_X86_32) && entry->start >= KERNEL_IMAGE_SIZE)
> 		return;
> 
> 	/* Ignore entries entirely below our minimum. */
>-	if (entry->addr + entry->size < minimum)
>+	if (entry->start + entry->size < minimum)
> 		return;
> 
> 	/* Ignore entries above memory limit */
>-	end = min(entry->size + entry->addr, mem_limit);
>-	if (entry->addr >= end)
>+	end = min(entry->size + entry->start, mem_limit);
>+	if (entry->start >= end)
> 		return;
>-	cur_entry.addr = entry->addr;
>-	cur_entry.size = end - entry->addr;
>+	cur_entry.start = entry->start;
>+	cur_entry.size = end - entry->start;
> 
>-	region.start = cur_entry.addr;
>+	region.start = cur_entry.start;
> 	region.size = cur_entry.size;
> 
> 	/* Give up if slot area array is full. */
>@@ -522,7 +518,7 @@ static void process_e820_entry(struct boot_e820_entry *entry,
> 		region.start = ALIGN(region.start, CONFIG_PHYSICAL_ALIGN);
> 
> 		/* Did we raise the address above this e820 region? */
>-		if (region.start > cur_entry.addr + cur_entry.size)
>+		if (region.start > cur_entry.start + cur_entry.size)
> 			return;
> 
> 		/* Reduce size by any delta from the original address. */
>@@ -562,12 +558,31 @@ static void process_e820_entry(struct boot_e820_entry *entry,
> 	}
> }
> 
>-static unsigned long find_random_phys_addr(unsigned long minimum,
>-					   unsigned long image_size)
>+static void process_e820_entry(unsigned long minimum,unsigned long image_size)
> {
> 	int i;
>-	unsigned long addr;
>+	struct mem_vector region;
>+	struct boot_e820_entry *entry;
>+
>+	/* Verify potential e820 positions, appending to slots list. */
>+        for (i = 0; i < boot_params->e820_entries; i++) {
>+                entry = &boot_params->e820_table[i];
>+                /* Skip non-RAM entries. */
>+                if (entry->type != E820_TYPE_RAM)
>+                        continue;
>+                region.start = entry->addr;
>+                region.size = entry->size;
>+                process_mem_region(&region, minimum, image_size);
>+                if (slot_area_index == MAX_SLOT_AREA) {
>+                        debug_putstr("Aborted e820 scan (slot_areas full)!\n");
>+                        break;
>+                }
>+        }
>+}
> 
>+static unsigned long find_random_phys_addr(unsigned long minimum,
>+					   unsigned long image_size)
>+{
> 	/* Check if we had too many memmaps. */
> 	if (memmap_too_large) {
> 		debug_putstr("Aborted e820 scan (more than 4 memmap= args)!\n");
>@@ -577,16 +592,7 @@ static unsigned long find_random_phys_addr(unsigned long minimum,
> 	/* Make sure minimum is aligned. */
> 	minimum = ALIGN(minimum, CONFIG_PHYSICAL_ALIGN);
> 
>-	/* Verify potential e820 positions, appending to slots list. */
>-	for (i = 0; i < boot_params->e820_entries; i++) {
>-		process_e820_entry(&boot_params->e820_table[i], minimum,
>-				   image_size);
>-		if (slot_area_index == MAX_SLOT_AREA) {
>-			debug_putstr("Aborted e820 scan (slot_areas full)!\n");
>-			break;
>-		}
>-	}
>-
>+	process_e820_entry(minimum, image_size);
> 	return slots_fetch_random();
> }
> 
>-- 
>2.5.5
>
>
>

Re: [RFC][PATCH 0/2] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if existed

[Chao Fan]

Hi all,

After testing this issue for 100 times in total, I think this patchset
works well.

The size of physical memory in my test machine is 229G, the size of
mirror region is 26G. In the 100 times, 50 times are with this patchset,
50 times are without it.

Here is my test result:

------------------------------------------------
            |total times|in non-mirror|in mirror
------------|-----------|-------------|---------
before patch|     50    |      41     |   9
------------|-----------|-------------|---------
with patch  |     50    |       0     |  50
------------------------------------------------

Firstly, I add the earlyprintk to get efi map when walking the efi map.
Then get the range of mirror regions.
In kaslr.c, add the earlyprintk to get random_addr in function
choose_random_location, find_random_phys_addr. Then check if the address
in which is choosen to extract kernel is in mirror region.

If there are any problems, please let me know.

Thanks,
Chao Fan

On Thu, Jun 15, 2017 at 03:52:47PM +0800, Baoquan He wrote:
>Our customer reported that Kernel text may be located on non-mirror
>region (movable zone) when both address range mirroring feature and
>KASLR are enabled.
>
>The functions of address range mirroring feature are as follows.
>- The physical memory region whose descriptors in EFI memory map have
>  EFI_MEMORY_MORE_RELIABLE attribute (bit: 16) are mirrored
>- The function arranges such mirror region into normal zone and other region
>  into movable zone in order to locate kernel code and data on mirror region
>
>So we need restrict kernel to be located inside mirror region if it
>is existed.
>
>The method is very simple. If efi is enabled, just iterate all efi
>memory map and pick up mirror region to process for adding candidate
>of slot. If efi disabled or no mirror region existed, still process
>e820 memory map. This won't bring much efficiency loss, at worst we
>just go through all efi memory maps and found no mirror.
>
>One question:
>From code, though mirror regions are existed, they are meaningful only
>if kernelcore=mirror kernel option is specified. Not sure if my understanding
>is correct.
>
>NOTE:
>I haven't got a machine with efi mirror region enabled, so only test the
>e820 map processing case and the case of no mirror region on efi machine.
>So set this as a RFC patchset, will post formal one after above question
>is made clear and mirror issue test passed.
>
>Baoquan He (2):
>  x86/boot/KASLR: Adapt process_e820_entry for all kinds of memory map
>  x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if
>    existed
>
> arch/x86/boot/compressed/kaslr.c | 129 +++++++++++++++++++++++++++++++--------
> 1 file changed, 104 insertions(+), 25 deletions(-)
>
>-- 
>2.5.5
>
>
>

Re: [RFC][PATCH 0/2] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if existed

[Baoquan He]

On 06/22/17 at 11:10am, Chao Fan wrote:
> Hi all,
> 
> After testing this issue for 100 times in total, I think this patchset
> works well.

Thanks for your effort, Chao!

Then I will repost with some modification according to the kbuild test report
and your comment, and thanks again for that. Maybe next week, there is 
urgent rhel bug now I am trying to fix.

Thanks
Baoquan

> 
> The size of physical memory in my test machine is 229G, the size of
> mirror region is 26G. In the 100 times, 50 times are with this patchset,
> 50 times are without it.
> 
> Here is my test result:
> 
> ------------------------------------------------
>             |total times|in non-mirror|in mirror
> ------------|-----------|-------------|---------
> before patch|     50    |      41     |   9
> ------------|-----------|-------------|---------
> with patch  |     50    |       0     |  50
> ------------------------------------------------
> 
> Firstly, I add the earlyprintk to get efi map when walking the efi map.
> Then get the range of mirror regions.
> In kaslr.c, add the earlyprintk to get random_addr in function
> choose_random_location, find_random_phys_addr. Then check if the address
> in which is choosen to extract kernel is in mirror region.
> 
> If there are any problems, please let me know.
> 
> Thanks,
> Chao Fan
> 
> On Thu, Jun 15, 2017 at 03:52:47PM +0800, Baoquan He wrote:
> >Our customer reported that Kernel text may be located on non-mirror
> >region (movable zone) when both address range mirroring feature and
> >KASLR are enabled.
> >
> >The functions of address range mirroring feature are as follows.
> >- The physical memory region whose descriptors in EFI memory map have
> >  EFI_MEMORY_MORE_RELIABLE attribute (bit: 16) are mirrored
> >- The function arranges such mirror region into normal zone and other region
> >  into movable zone in order to locate kernel code and data on mirror region
> >
> >So we need restrict kernel to be located inside mirror region if it
> >is existed.
> >
> >The method is very simple. If efi is enabled, just iterate all efi
> >memory map and pick up mirror region to process for adding candidate
> >of slot. If efi disabled or no mirror region existed, still process
> >e820 memory map. This won't bring much efficiency loss, at worst we
> >just go through all efi memory maps and found no mirror.
> >
> >One question:
> >From code, though mirror regions are existed, they are meaningful only
> >if kernelcore=mirror kernel option is specified. Not sure if my understanding
> >is correct.
> >
> >NOTE:
> >I haven't got a machine with efi mirror region enabled, so only test the
> >e820 map processing case and the case of no mirror region on efi machine.
> >So set this as a RFC patchset, will post formal one after above question
> >is made clear and mirror issue test passed.
> >
> >Baoquan He (2):
> >  x86/boot/KASLR: Adapt process_e820_entry for all kinds of memory map
> >  x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if
> >    existed
> >
> > arch/x86/boot/compressed/kaslr.c | 129 +++++++++++++++++++++++++++++++--------
> > 1 file changed, 104 insertions(+), 25 deletions(-)
> >
> >-- 
> >2.5.5
> >
> >
> >
> 
> 

Re: [RFC][PATCH 0/2] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if existed

[Chao Fan]

On Thu, Jun 22, 2017 at 11:20:32AM +0800, Baoquan He wrote:
>On 06/22/17 at 11:10am, Chao Fan wrote:
>> Hi all,
>> 
>> After testing this issue for 100 times in total, I think this patchset
>> works well.
>
>Thanks for your effort, Chao!

No problem, and many thanks for your patch, Bao!

>
>Then I will repost with some modification according to the kbuild test report
>and your comment, and thanks again for that. Maybe next week, there is 
>urgent rhel bug now I am trying to fix.

That's OK if next week. But of course, the earlier to repost, the
better for us.
Thank you again for the patch.

Thanks,
Chao Fan

>
>Thanks
>Baoquan
>
>> 
>> The size of physical memory in my test machine is 229G, the size of
>> mirror region is 26G. In the 100 times, 50 times are with this patchset,
>> 50 times are without it.
>> 
>> Here is my test result:
>> 
>> ------------------------------------------------
>>             |total times|in non-mirror|in mirror
>> ------------|-----------|-------------|---------
>> before patch|     50    |      41     |   9
>> ------------|-----------|-------------|---------
>> with patch  |     50    |       0     |  50
>> ------------------------------------------------
>> 
>> Firstly, I add the earlyprintk to get efi map when walking the efi map.
>> Then get the range of mirror regions.
>> In kaslr.c, add the earlyprintk to get random_addr in function
>> choose_random_location, find_random_phys_addr. Then check if the address
>> in which is choosen to extract kernel is in mirror region.
>> 
>> If there are any problems, please let me know.
>> 
>> Thanks,
>> Chao Fan
>> 
>> On Thu, Jun 15, 2017 at 03:52:47PM +0800, Baoquan He wrote:
>> >Our customer reported that Kernel text may be located on non-mirror
>> >region (movable zone) when both address range mirroring feature and
>> >KASLR are enabled.
>> >
>> >The functions of address range mirroring feature are as follows.
>> >- The physical memory region whose descriptors in EFI memory map have
>> >  EFI_MEMORY_MORE_RELIABLE attribute (bit: 16) are mirrored
>> >- The function arranges such mirror region into normal zone and other region
>> >  into movable zone in order to locate kernel code and data on mirror region
>> >
>> >So we need restrict kernel to be located inside mirror region if it
>> >is existed.
>> >
>> >The method is very simple. If efi is enabled, just iterate all efi
>> >memory map and pick up mirror region to process for adding candidate
>> >of slot. If efi disabled or no mirror region existed, still process
>> >e820 memory map. This won't bring much efficiency loss, at worst we
>> >just go through all efi memory maps and found no mirror.
>> >
>> >One question:
>> >From code, though mirror regions are existed, they are meaningful only
>> >if kernelcore=mirror kernel option is specified. Not sure if my understanding
>> >is correct.
>> >
>> >NOTE:
>> >I haven't got a machine with efi mirror region enabled, so only test the
>> >e820 map processing case and the case of no mirror region on efi machine.
>> >So set this as a RFC patchset, will post formal one after above question
>> >is made clear and mirror issue test passed.
>> >
>> >Baoquan He (2):
>> >  x86/boot/KASLR: Adapt process_e820_entry for all kinds of memory map
>> >  x86/boot/KASLR: Restrict kernel to be randomized in mirror regions if
>> >    existed
>> >
>> > arch/x86/boot/compressed/kaslr.c | 129 +++++++++++++++++++++++++++++++--------
>> > 1 file changed, 104 insertions(+), 25 deletions(-)
>> >
>> >-- 
>> >2.5.5
>> >
>> >
>> >
>> 
>> 
>
>