Re: [PATCH v5 2/3] KASLR: Handle memory limit specified by memmap and mem option

[Baoquan He <bhe@redhat.com>]

On 05/16/17 at 09:43am, Dou Liyang wrote:
> 
> 
> 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(-)
> > > > 
> > > >  	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 for testing and looking into this patchset, Liyang. I understand
your worry, this patch is trying to have the same behaviour as in
arch/x86/kernel/e820.c. Seems the liability of specifying them right lays
on users' shoulder.

Thanks
Baoquan