Re: [PATCH] mm/mremap: Fail map duplication attempts for private mappings

[Michal Hocko]

On Wed 19-07-17 09:39:50, Mike Kravetz wrote:
> On 07/13/2017 12:11 PM, Vlastimil Babka wrote:
> > [+CC linux-api]
> > 
> > On 07/13/2017 05:58 PM, Mike Kravetz wrote:
> >> mremap will create a 'duplicate' mapping if old_size == 0 is
> >> specified.  Such duplicate mappings make no sense for private
> >> mappings.  If duplication is attempted for a private mapping,
> >> mremap creates a separate private mapping unrelated to the
> >> original mapping and makes no modifications to the original.
> >> This is contrary to the purpose of mremap which should return
> >> a mapping which is in some way related to the original.
> >>
> >> Therefore, return EINVAL in the case where if an attempt is
> >> made to duplicate a private mapping.
> >>
> >> Signed-off-by: Mike Kravetz <mike.kravetz@xxxxxxxxxx>
> > 
> > Acked-by: Vlastimil Babka <vbabka@xxxxxxx>
> 
> After considering Michal's concerns with follow on patch, it appears
> this patch provides the most desired behavior.  Any other concerns
> or issues with this patch?

Maybe we should add a pr_warn_once to make users aware that this is no
longer supported.

> If this moves forward, I will create man page updates to describe the
> mremap(old_size == 0) behavior.
> 
> -- 
> Mike Kravetz
> 
> > 
> >> ---
> >>  mm/mremap.c | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/mm/mremap.c b/mm/mremap.c
> >> index cd8a1b1..076f506 100644
> >> --- a/mm/mremap.c
> >> +++ b/mm/mremap.c
> >> @@ -383,6 +383,13 @@ static struct vm_area_struct *vma_to_resize(unsigned long addr,
> >>  	if (!vma || vma->vm_start > addr)
> >>  		return ERR_PTR(-EFAULT);
> >>  
> >> +	/*
> >> +	 * !old_len  is a special case where a mapping is 'duplicated'.
> >> +	 * Do not allow this for private mappings.

Do not allow this for private mappings because we have never really
duplicated the range for those so the new VMA is a fresh one unrelated
to the original one which breaks mremap semantic. While we can do that
there doesn't seem to be any existing usecase currently.

> >> +	 */
> >> +	if (!old_len && !(vma->vm_flags & (VM_SHARED | VM_MAYSHARE)))
> >> +		return ERR_PTR(-EINVAL);
> >> +
> >>  	if (is_vm_hugetlb_page(vma))
> >>  		return ERR_PTR(-EINVAL);
> >>  
> >>
> > 

-- 
Michal Hocko
SUSE Labs