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

From: Baoquan He
Date: Wed Jan 04 2017 - 20:00:53 EST


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