Re: [PATCH 1/3] mm: shift VM_GROWS* check from mmap_region() to do_mmap_pgoff()

[Andrew Morton <akpm@linux-foundation.org>]

On Sun, 14 Jul 2013 18:54:51 +0200 Oleg Nesterov <oleg@redhat.com> wrote:

> mmap() doesn't allow the non-anonymous mappings with VM_GROWS* bit set.
> In particular this means that mmap_region()->vma_merge(file, vm_flags)
> must always fail if vm_flags & VM_GROWS. So it does not make sense to
> check VM_GROWS* after we already allocated the new vma, the only caller,
> do_mmap_pgoff(), which can pass this flag can do the check itself.
> 
> And this looks a bit more correct, mmap_region() already unmapped the
> old mapping at this stage. But if mmap() is going to fail, it should
> avoid do_munmap() if possible.
> 
> ...
>
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1327,6 +1327,9 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned long addr,
>  		}
>  	}
>  
> +	/* Only MAP_PRIVATE|MAP_ANONYMOUS can use MAP_GROWS */
> +	if ((vm_flags & VM_MAYSHARE) && (vm_flags & (VM_GROWSDOWN|VM_GROWSUP)))

That is of course vm_flags&(VM_MAYSHARE|VM_GROWSDOWN|VM_GROWSUP), but
that perhaps is less clear.

> +		return -EINVAL;

I had to stare for a while but yes, the change looks OK to me.