linux-kernel - Re: [PATCH v4] x86: fix kaslr and memmap collision

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 5 Jan 2017 09:00:35 +0800
From:   Baoquan He <bhe@...hat.com>
To:     Dave Jiang <dave.jiang@...el.com>
Cc:     tglx@...utronix.de, mingo@...hat.com, hpa@...or.com,
        linux-nvdimm@...1.01.org, x86@...nel.org, david@...morbit.com,
        linux-kernel@...r.kernel.org, dan.j.williams@...el.com
Subject: Re: [PATCH v4] x86: fix kaslr and memmap collision

On 01/04/17 at 10:06am, Dave Jiang wrote:
> On 01/03/2017 07:37 PM, Baoquan He wrote:
> >>  #include <generated/compile.h>
> >>  #include <linux/module.h>
> >> @@ -61,9 +62,16 @@ enum mem_avoid_index {
> >>  	MEM_AVOID_INITRD,
> >>  	MEM_AVOID_CMDLINE,
> >>  	MEM_AVOID_BOOTPARAMS,
> >> +	MEM_AVOID_MEMMAP1,
> >> +	MEM_AVOID_MEMMAP2,
> >> +	MEM_AVOID_MEMMAP3,
> >> +	MEM_AVOID_MEMMAP4,
> > 
> > This looks not good. Could it be done like fixed_addresses?
> > Something like:
> > 
> > 	MEM_AVOID_MEMMAP_BEGIN,
> > 	MEM_AVOID_MEMMAP_BEGIN + MEM_AVOID_MAX -1,
> > 
> > Please point it out to me if there's some existing code in kernel like
> > your way, I can also accept it.
> 
> I think you mean:
> MEM_AVOID_MEMMAP_BEGIN + MAX_MEMMAP_REGIONS - 1

Ah, yes. Sorry for this.

> 
> I will change
> 
> 
> > 
> >>  	MEM_AVOID_MAX,
> >>  };
> >>  
> >> +/* only supporting at most 4 memmap regions with kaslr */
> > And here, "Only supporting at most 4 un-usable memmap regions with kaslr"?
> > Surely this is based on if you will ignore the usable memory and do not
> > store it as 0. And also the log need be changed too accordingly.
> >> +#define MAX_MEMMAP_REGIONS	4
...
> >> +static int
> >> +parse_memmap(char *p, unsigned long long *start, unsigned long long *size)
> >> +{
> >> +	char *oldp;
> >> +
> >> +	if (!p)
> >> +		return -EINVAL;
> >> +
> >> +	/* we don't care about this option here */
> >> +	if (!strncmp(p, "exactmap", 8))
> >> +		return -EINVAL;
> >> +
> >> +	oldp = p;
> >> +	*size = _memparse(p, &p);
> >> +	if (p == oldp)
> >> +		return -EINVAL;
> >> +
> >> +	switch (*p) {
> >> +	case '@':
> >> +		/* skip this region, usable */
> >> +		*start = 0;
> >> +		*size = 0;
> >> +		return 0;
> > How about direclty return if nn@ss? Seems no need to waste one mem avoid
> > region slot. In fact even amount of usable memory regions are provided to
> > 100, it won't impact that you want to specify a reserve memmap region if
> > you skip it direclty. Personal opinion.
> 
> We are not wasting the slot. If you look mem_avoid_memmap() where I call
> the function, it will skip with a continue if size == 0 without
> incrementing the 'i' counter. That will skip all the nn@ss regions
> without counting against the max avoid mapping.

Yes, indeed. Sorry, I didn't read the code carefully.

Thanks
Baoquan