Re: [PATCH] mm/mmap.c: remove incorrect MAP_FIXED flag comparison from mmap_region

[Piotr Kwapulinski <kwapulinski.piotr@gmail.com>]

On Tue, Nov 17, 2015 at 04:52:51PM -0800, Andrew Morton wrote:
> On Tue, 17 Nov 2015 18:26:38 +0100 Piotr Kwapulinski <kwapulinski.piotr@gmail.com> wrote:
> 
> > The following flag comparison in mmap_region is not fully correct:
> > 
> > if (!(vm_flags & MAP_FIXED))
> > 
> > The vm_flags should not be compared with MAP_FIXED (0x10). It is a bit
> > confusing. This condition is almost always true since VM_MAYREAD (0x10)
> > flag is almost always set by default. This patch removes this condition.
> > 
> > ...
> >
> > --- a/mm/mmap.c
> > +++ b/mm/mmap.c
> > @@ -1547,13 +1547,6 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
> >  	if (!may_expand_vm(mm, len >> PAGE_SHIFT)) {
> >  		unsigned long nr_pages;
> >  
> > -		/*
> > -		 * MAP_FIXED may remove pages of mappings that intersects with
> > -		 * requested mapping. Account for the pages it would unmap.
> > -		 */
> > -		if (!(vm_flags & MAP_FIXED))
> > -			return -ENOMEM;
> > -
> >  		nr_pages = count_vma_pages_range(mm, addr, addr + len);
> >  
> >  		if (!may_expand_vm(mm, (len >> PAGE_SHIFT) - nr_pages))
> 
> That looks simpler.
> 
> However the changelog doesn't describe the end-user visible effects of
> the bug, as changelogs should always do.  Presumably this is causing
> incorrect ENOMEM reporting due to RLIMIT_AS being exceeded, but this
> isn't very specific.
> 
> So can you please fill in the details here?  Such info is needed when
> deciding which kernel version(s) need the fix.
> 
> Thanks.

The first patch has got a user visible effect and it fixes the
real issue (corner case one). The second patch has no user visible effect.
It just removes the code that makes no sense. The second patch has
been created in case the first patch was not going to be accepted.
I will send both patches again to let you choose which one you preffer.
This time the patches will contain the more clear changelog containing
the user visible effect. 

Thanks.
---
Piotr Kwapulinski

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>