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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170720082058.GF9058@dhcp22.suse.cz>
Date:   Thu, 20 Jul 2017 10:20:58 +0200
From:   Michal Hocko <mhocko@...nel.org>
To:     Mike Kravetz <mike.kravetz@...cle.com>
Cc:     Vlastimil Babka <vbabka@...e.cz>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Aaron Lu <aaron.lu@...el.com>,
        "Kirill A . Shutemov" <kirill.shutemov@...ux.intel.com>,
        Anshuman Khandual <khandual@...ux.vnet.ibm.com>,
        Linux API <linux-api@...r.kernel.org>
Subject: Re: [PATCH] mm/mremap: Fail map duplication attempts for private
 mappings

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@...cle.com>
> > 
> > Acked-by: Vlastimil Babka <vbabka@...e.cz>
> 
> 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