Messages in this thread | | | Subject | Re: [PATCH v5 2/3] KASLR: Handle memory limit specified by memmap and mem option | From | Dou Liyang <> | Date | Tue, 16 May 2017 09:43:45 +0800 |
| |
At 05/16/2017 09:12 AM, Baoquan He wrote: > On 05/16/17 at 08:56am, Dou Liyang wrote: >> Hi Baoquan, >> >> At 05/13/2017 01:46 PM, Baoquan He wrote: >>> Option mem= will limit the max address a system can use and any memory >>> region above the limit will be removed. >>> >>> Furthermore, memmap=nn[KMG] which has no offset specified has the same >>> behaviour as mem=. >>> >>> KASLR needs to consider this when choosing the random position for >>> decompressing the kernel. Do it now. >>> >>> Signed-off-by: Baoquan He <bhe@redhat.com> >>> Tested-by: Masayoshi Mizuma <m.mizuma@jp.fujitsu.com> >>> --- >>> arch/x86/boot/compressed/kaslr.c | 68 +++++++++++++++++++++++++++++----------- >>> 1 file changed, 50 insertions(+), 18 deletions(-) >>> >>> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c >>> index 106e13b..e0eba12 100644 >>> --- a/arch/x86/boot/compressed/kaslr.c >>> +++ b/arch/x86/boot/compressed/kaslr.c >>> @@ -88,6 +88,10 @@ struct mem_vector { >>> static bool memmap_too_large; >>> >>> >>> +/* Store memory limit specified by "mem=nn[KMG]" or "memmap=nn[KMG]" */ >>> +unsigned long long mem_limit = ULLONG_MAX; >>> + >>> + >>> enum mem_avoid_index { >>> MEM_AVOID_ZO_RANGE = 0, >>> MEM_AVOID_INITRD, >>> @@ -138,16 +142,23 @@ parse_memmap(char *p, unsigned long long *start, unsigned long long *size) >>> return -EINVAL; >>> >>> switch (*p) { >>> - case '@': >>> - /* Skip this region, usable */ >>> - *start = 0; >>> - *size = 0; >>> - return 0; >>> case '#': >>> case '$': >>> case '!': >>> *start = memparse(p + 1, &p); >>> return 0; >>> + case '@': >>> + /* memmap=nn@ss specifies usable region, should be skipped */ >>> + *size = 0; >>> + /* Fall through */ >>> + default: >>> + /* >>> + * If w/o offset, only size specified, memmap=nn[KMG] has the >>> + * same behaviour as mem=nn[KMG]. It limits the max address >>> + * system can use. Region above the limit should be avoided. >>> + */ >>> + *start = 0; >>> + return 0; >>> } >>> >>> return -EINVAL; >>> @@ -173,9 +184,14 @@ static void mem_avoid_memmap(char *str) >>> if (rc < 0) >>> break; >>> str = k; >>> - /* A usable region that should not be skipped */ >>> - if (size == 0) >>> + >>> + if (start == 0) { >>> + /* Store the specified memory limit if size > 0 */ >>> + if (size > 0) >>> + mem_limit = size; >> >> Baoquan, >> >> I am not sure about setting the value of mem_limit to mem_size directly. >> >> If the command line has both the "memmap" and "mem", such as >> ... mem=2G memmap=4G ... >> >> ...in that code, the mem_limit may be 4G not 2G. > > No, could you tell why you want to add both "memmap=nnKMG" and "mem=" at > the same time? As you sid, what if I add "mem=4G mem=2G mem=1G"?
Just for testing :)
Ok, thanks, I see. We should be responsible for our command line. don't need to consider with these situations in kernel.
Thanks, Liyang. > >> >> In my opinion, How about following: >> >> mem_limit = mem_limit > mem_size ? mem_size : mem_limit; >> >>> + >>> continue; >>> + } >>> >>> mem_avoid[MEM_AVOID_MEMMAP_BEGIN + i].start = start; >>> mem_avoid[MEM_AVOID_MEMMAP_BEGIN + i].size = size; >>> @@ -187,19 +203,15 @@ static void mem_avoid_memmap(char *str) >>> memmap_too_large = true; >>> } >>> >>> - >>> -/* >>> - * handle_mem_memmap will also cover 'mem=' issue in next patch. Will remove >>> - * this note later. >>> - */ >>> static int handle_mem_memmap(void) >>> { >>> char *args = (char *)get_cmd_line_ptr(); >>> size_t len = strlen((char *)args); >>> char *tmp_cmdline; >>> char *param, *val; >>> + u64 mem_size; >>> >>> - if (!strstr(args, "memmap=")) >>> + if (!strstr(args, "memmap=") && !strstr(args, "mem=")) >>> return 0; >>> >>> tmp_cmdline = malloc(len + 1); >>> @@ -222,8 +234,20 @@ static int handle_mem_memmap(void) >>> return -1; >>> } >>> >>> - if (!strcmp(param, "memmap")) >>> + if (!strcmp(param, "memmap")) { >>> mem_avoid_memmap(val); >>> + } else if (!strcmp(param, "mem")) { >>> + char *p = val; >>> + >>> + if (!strcmp(p, "nopentium")) >>> + continue; >>> + mem_size = memparse(p, &p); >>> + if (mem_size == 0) { >>> + free(tmp_cmdline); >>> + return -EINVAL; >>> + } >>> + mem_limit = mem_size; >> >> The same as above. >> >> Thanks, >> Liyang. >> >>> + } >>> } >>> >>> free(tmp_cmdline); >>> @@ -460,7 +484,8 @@ static void process_e820_entry(struct boot_e820_entry *entry, >>> { >>> struct mem_vector region, overlap; >>> struct slot_area slot_area; >>> - unsigned long start_orig; >>> + unsigned long start_orig, end; >>> + struct boot_e820_entry cur_entry; >>> >>> /* Skip non-RAM entries. */ >>> if (entry->type != E820_TYPE_RAM) >>> @@ -474,8 +499,15 @@ static void process_e820_entry(struct boot_e820_entry *entry, >>> if (entry->addr + entry->size < minimum) >>> return; >>> >>> - region.start = entry->addr; >>> - region.size = entry->size; >>> + /* Ignore entries above memory limit */ >>> + end = min(entry->size + entry->addr, mem_limit); >>> + if (entry->addr >= end) >>> + return; >>> + cur_entry.addr = entry->addr; >>> + cur_entry.size = end - entry->addr; >>> + >>> + region.start = cur_entry.addr; >>> + region.size = cur_entry.size; >>> >>> /* Give up if slot area array is full. */ >>> while (slot_area_index < MAX_SLOT_AREA) { >>> @@ -489,7 +521,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 > entry->addr + entry->size) >>> + if (region.start > cur_entry.addr + cur_entry.size) >>> return; >>> >>> /* Reduce size by any delta from the original address. */ >>> >> >> > > >
| |