From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-20.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, HTML_MESSAGE,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9D8ECC433E4 for ; Tue, 23 Mar 2021 18:27:07 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 1566A619C1 for ; Tue, 23 Mar 2021 18:27:07 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1566A619C1 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 9188E6B0288; Tue, 23 Mar 2021 14:27:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8EF3F6B028A; Tue, 23 Mar 2021 14:27:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 769576B028B; Tue, 23 Mar 2021 14:27:06 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0172.hostedemail.com [216.40.44.172]) by kanga.kvack.org (Postfix) with ESMTP id 56F996B0288 for ; Tue, 23 Mar 2021 14:27:06 -0400 (EDT) Received: from smtpin09.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id F3342180CA1A0 for ; Tue, 23 Mar 2021 18:27:05 +0000 (UTC) X-FDA: 77951970852.09.A0A6A85 Received: from mail-ej1-f46.google.com (mail-ej1-f46.google.com [209.85.218.46]) by imf13.hostedemail.com (Postfix) with ESMTP id 20DEDE0001AF for ; Tue, 23 Mar 2021 18:27:04 +0000 (UTC) Received: by mail-ej1-f46.google.com with SMTP id kt15so19220153ejb.12 for ; Tue, 23 Mar 2021 11:27:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=n0I6Xi8JuekuApD58s7TA4PR9m7AwRVItx5kymV/LJQ=; b=vkNSQhnDiZ0KPYbtnZLayzsqLWXi9MvsoIvDDBDfQexxqJEvZSjZWY+tT+glh6wbgh oDYnVDYdQPZJtPxfccVWUTTIXpHii2drEx1CdYCMA2JojUH4Yp/baDRXQcyzmu2r2ayh nMQWynKLQNDfAFCkp2a2Q39Kgx6TihXlH/5XNYPrJ3OX96pVYeW6echOMphQFiOX481Q c11AV/JavXP9YSDvrtZLC6Sr7ZolAKGmHaJovg3lqBcB/ftVWUYyz5caUyDMQfFvUSL0 nkKwMoQw4m6iqW3JhfM/Nf8KumznuPCwx0j9nbPYPrjEB635w7a7H+vfIFFnJRi5sRLh XPFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=n0I6Xi8JuekuApD58s7TA4PR9m7AwRVItx5kymV/LJQ=; b=NePbNTCv8E/VHWpcKPZO4I42/at9Tqo2oLMdlsL5ByvsOP1BsIzoab9bTVEmXQ/5x+ 6ABDoetNygv4DoGLOzkMba+P8ZKgTGMWFab5+I75e8KoIvNfJ5aFJjM3xJ1Id6e6P7vQ ZT6k05GxFtdANFaP13kc/WSw1MPkmtYqrN01ru5sfuc223nkrjzdfszsNgcoCrhGPCzZ FiTx1YiJrdYMvIfOpP2m46kJ2GbR+OSI0IY5Ce7+z09ar49glG/tW333gi5OdrWaglVR D9/cVW2DJzk4wjxkA1td0R8XsC2ctlycWXh8VRqaVKaj7/lb5ojpBc7ge70WemTWwCHk 6PZw== X-Gm-Message-State: AOAM530qgOX65neJny7joJJOvBAIJZU1d/QgQE/zaYQkhaY5ZlHksyhR m7tUtohlrLw6+1i0AdJwIL/YhNAkOH+rv0L/JbQ+YQ== X-Google-Smtp-Source: ABdhPJwcn02TixRzjYBZrf5PVXgMs40BljKXLAXKBTyP3GNkkWCsJrRW/teBSlcuP7wFLD1XWfPF0c5i80AJZIaXhLs= X-Received: by 2002:a17:906:52d0:: with SMTP id w16mr6181857ejn.172.1616524023976; Tue, 23 Mar 2021 11:27:03 -0700 (PDT) MIME-Version: 1.0 References: <20210303175235.3308220-1-bgeffon@google.com> <20210323162611.2398613-1-bgeffon@google.com> <20210323162611.2398613-2-bgeffon@google.com> In-Reply-To: From: Brian Geffon Date: Tue, 23 Mar 2021 11:26:27 -0700 Message-ID: Subject: Re: [PATCH v4 2/3] Revert "mremap: don't allow MREMAP_DONTUNMAP on special_mappings and aio" To: Hugh Dickins Cc: Andrew Morton , Axel Rasmussen , Lokesh Gidra , Mike Rapoport , Peter Xu , "Michael S . Tsirkin" , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andy Lutomirski , Vlastimil Babka , Andrea Arcangeli , Sonny Rao , Minchan Kim , "Kirill A . Shutemov" , Dmitry Safonov , Michael Kerrisk , Alejandro Colomar Content-Type: multipart/alternative; boundary="00000000000038cd4505be38542f" X-Stat-Signature: iknmtybs9ojaw364mg7jkhu83mwd6ct8 X-Rspamd-Server: rspam02 X-Rspamd-Queue-Id: 20DEDE0001AF Received-SPF: none (google.com>: No applicable sender policy available) receiver=imf13; identity=mailfrom; envelope-from=""; helo=mail-ej1-f46.google.com; client-ip=209.85.218.46 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1616524024-113044 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: --00000000000038cd4505be38542f Content-Type: text/plain; charset="UTF-8" I apologize for that oversight, a new version with the updated commit message has been mailed. Brian On Tue, Mar 23, 2021 at 11:17 AM Hugh Dickins wrote: > On Tue, 23 Mar 2021, Brian Geffon wrote: > > > This reverts commit cd544fd1dc9293c6702fab6effa63dac1cc67e99. > > > > As discussed in [1] this commit was a no-op because the mapping type was > > checked in vma_to_resize before move_vma is ever called. This meant that > > vm_ops->mremap() would never be called on such mappings. Furthermore, > > we've since expanded support of MREMAP_DONTUNMAP to non-anonymous > > mappings, and these special mappings are still protected by the existing > > check of !VM_DONTEXPAND and !VM_PFNMAP which will result in a -EFAULT. > > No, those two lines still describe an earlier version, they should say: > "mappings, and these special mappings are now protected by a check of > !VM_DONTEXPAND and !VM_PFNMAP which will result in a -EINVAL." > > > > > 1. https://lkml.org/lkml/2020/12/28/2340 > > > > Signed-off-by: Brian Geffon > > Acked-by: Hugh Dickins > > --- > > arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 2 +- > > fs/aio.c | 5 +---- > > include/linux/mm.h | 2 +- > > mm/mmap.c | 6 +----- > > mm/mremap.c | 2 +- > > 5 files changed, 5 insertions(+), 12 deletions(-) > > > > diff --git a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c > b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c > > index e916646adc69..0daf2f1cf7a8 100644 > > --- a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c > > +++ b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c > > @@ -1458,7 +1458,7 @@ static int pseudo_lock_dev_release(struct inode > *inode, struct file *filp) > > return 0; > > } > > > > -static int pseudo_lock_dev_mremap(struct vm_area_struct *area, unsigned > long flags) > > +static int pseudo_lock_dev_mremap(struct vm_area_struct *area) > > { > > /* Not supported */ > > return -EINVAL; > > diff --git a/fs/aio.c b/fs/aio.c > > index 1f32da13d39e..76ce0cc3ee4e 100644 > > --- a/fs/aio.c > > +++ b/fs/aio.c > > @@ -323,16 +323,13 @@ static void aio_free_ring(struct kioctx *ctx) > > } > > } > > > > -static int aio_ring_mremap(struct vm_area_struct *vma, unsigned long > flags) > > +static int aio_ring_mremap(struct vm_area_struct *vma) > > { > > struct file *file = vma->vm_file; > > struct mm_struct *mm = vma->vm_mm; > > struct kioctx_table *table; > > int i, res = -EINVAL; > > > > - if (flags & MREMAP_DONTUNMAP) > > - return -EINVAL; > > - > > spin_lock(&mm->ioctx_lock); > > rcu_read_lock(); > > table = rcu_dereference(mm->ioctx_table); > > diff --git a/include/linux/mm.h b/include/linux/mm.h > > index 64a71bf20536..ecdc6e8dc5af 100644 > > --- a/include/linux/mm.h > > +++ b/include/linux/mm.h > > @@ -570,7 +570,7 @@ struct vm_operations_struct { > > void (*close)(struct vm_area_struct * area); > > /* Called any time before splitting to check if it's allowed */ > > int (*may_split)(struct vm_area_struct *area, unsigned long addr); > > - int (*mremap)(struct vm_area_struct *area, unsigned long flags); > > + int (*mremap)(struct vm_area_struct *area); > > /* > > * Called by mprotect() to make driver-specific permission > > * checks before mprotect() is finalised. The VMA must not > > diff --git a/mm/mmap.c b/mm/mmap.c > > index 3f287599a7a3..9d7651e4e1fe 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -3403,14 +3403,10 @@ static const char *special_mapping_name(struct > vm_area_struct *vma) > > return ((struct vm_special_mapping *)vma->vm_private_data)->name; > > } > > > > -static int special_mapping_mremap(struct vm_area_struct *new_vma, > > - unsigned long flags) > > +static int special_mapping_mremap(struct vm_area_struct *new_vma) > > { > > struct vm_special_mapping *sm = new_vma->vm_private_data; > > > > - if (flags & MREMAP_DONTUNMAP) > > - return -EINVAL; > > - > > if (WARN_ON_ONCE(current->mm != new_vma->vm_mm)) > > return -EFAULT; > > > > diff --git a/mm/mremap.c b/mm/mremap.c > > index db5b8b28c2dd..d22629ff8f3c 100644 > > --- a/mm/mremap.c > > +++ b/mm/mremap.c > > @@ -545,7 +545,7 @@ static unsigned long move_vma(struct vm_area_struct > *vma, > > if (moved_len < old_len) { > > err = -ENOMEM; > > } else if (vma->vm_ops && vma->vm_ops->mremap) { > > - err = vma->vm_ops->mremap(new_vma, flags); > > + err = vma->vm_ops->mremap(new_vma); > > } > > > > if (unlikely(err)) { > > -- > > 2.31.0.rc2.261.g7f71774620-goog > > > > > --00000000000038cd4505be38542f Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
I apologize for that oversight, a new version with the upd= ated commit message has been mailed.

Brian

On T= ue, Mar 23, 2021 at 11:17 AM Hugh Dickins <hughd@google.com> wrote:
On Tue, 23 Mar 2021, Brian Geffon wrote:

> This reverts commit cd544fd1dc9293c6702fab6effa63dac1cc67e99.
>
> As discussed in [1] this commit was a no-op because the mapping type w= as
> checked in vma_to_resize before move_vma is ever called. This meant th= at
> vm_ops->mremap() would never be called on such mappings. Furthermor= e,
> we've since expanded support of MREMAP_DONTUNMAP to non-anonymous<= br> > mappings, and these special mappings are still protected by the existi= ng
> check of !VM_DONTEXPAND and !VM_PFNMAP which will result in a -EFAULT.=

No, those two lines still describe an earlier version, they should say:
"mappings, and these special mappings are now protected by a check of<= br> =C2=A0!VM_DONTEXPAND and !VM_PFNMAP which will result in a -EINVAL."
>
> 1. https://lkml.org/lkml/2020/12/28/2340
>
> Signed-off-by: Brian Geffon <bgeffon@google.com>
> Acked-by: Hugh Dickins <hughd@google.com>
> ---
>=C2=A0 arch/x86/kernel/cpu/resctrl/pseudo_lock.c | 2 +-
>=C2=A0 fs/aio.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | 5 +---- >=C2=A0 include/linux/mm.h=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | 2 +-
>=C2=A0 mm/mmap.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| 6 +----- >=C2=A0 mm/mremap.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| 2 +-
>=C2=A0 5 files changed, 5 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c b/arch/x86/kern= el/cpu/resctrl/pseudo_lock.c
> index e916646adc69..0daf2f1cf7a8 100644
> --- a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> +++ b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> @@ -1458,7 +1458,7 @@ static int pseudo_lock_dev_release(struct inode = *inode, struct file *filp)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0return 0;
>=C2=A0 }
>=C2=A0
> -static int pseudo_lock_dev_mremap(struct vm_area_struct *area, unsign= ed long flags)
> +static int pseudo_lock_dev_mremap(struct vm_area_struct *area)
>=C2=A0 {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0/* Not supported */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0return -EINVAL;
> diff --git a/fs/aio.c b/fs/aio.c
> index 1f32da13d39e..76ce0cc3ee4e 100644
> --- a/fs/aio.c
> +++ b/fs/aio.c
> @@ -323,16 +323,13 @@ static void aio_free_ring(struct kioctx *ctx) >=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>=C2=A0 }
>=C2=A0
> -static int aio_ring_mremap(struct vm_area_struct *vma, unsigned long = flags)
> +static int aio_ring_mremap(struct vm_area_struct *vma)
>=C2=A0 {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0struct file *file =3D vma->vm_file;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0struct mm_struct *mm =3D vma->vm_mm;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0struct kioctx_table *table;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0int i, res =3D -EINVAL;
>=C2=A0
> -=C2=A0 =C2=A0 =C2=A0if (flags & MREMAP_DONTUNMAP)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -EINVAL;
> -
>=C2=A0 =C2=A0 =C2=A0 =C2=A0spin_lock(&mm->ioctx_lock);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0rcu_read_lock();
>=C2=A0 =C2=A0 =C2=A0 =C2=A0table =3D rcu_dereference(mm->ioctx_table= );
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 64a71bf20536..ecdc6e8dc5af 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -570,7 +570,7 @@ struct vm_operations_struct {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0void (*close)(struct vm_area_struct * area);=
>=C2=A0 =C2=A0 =C2=A0 =C2=A0/* Called any time before splitting to check= if it's allowed */
>=C2=A0 =C2=A0 =C2=A0 =C2=A0int (*may_split)(struct vm_area_struct *area= , unsigned long addr);
> -=C2=A0 =C2=A0 =C2=A0int (*mremap)(struct vm_area_struct *area, unsign= ed long flags);
> +=C2=A0 =C2=A0 =C2=A0int (*mremap)(struct vm_area_struct *area);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0/*
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 * Called by mprotect() to make driver-speci= fic permission
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 * checks before mprotect() is finalised.=C2= =A0 =C2=A0The VMA must not
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 3f287599a7a3..9d7651e4e1fe 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -3403,14 +3403,10 @@ static const char *special_mapping_name(struct= vm_area_struct *vma)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0return ((struct vm_special_mapping *)vma->= ;vm_private_data)->name;
>=C2=A0 }
>=C2=A0
> -static int special_mapping_mremap(struct vm_area_struct *new_vma,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned long flags)
> +static int special_mapping_mremap(struct vm_area_struct *new_vma)
>=C2=A0 {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0struct vm_special_mapping *sm =3D new_vma-&g= t;vm_private_data;
>=C2=A0
> -=C2=A0 =C2=A0 =C2=A0if (flags & MREMAP_DONTUNMAP)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -EINVAL;
> -
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (WARN_ON_ONCE(current->mm !=3D new_vma= ->vm_mm))
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -EFAULT;<= br> >=C2=A0
> diff --git a/mm/mremap.c b/mm/mremap.c
> index db5b8b28c2dd..d22629ff8f3c 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -545,7 +545,7 @@ static unsigned long move_vma(struct vm_area_struc= t *vma,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (moved_len < old_len) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0err =3D -ENOMEM;=
>=C2=A0 =C2=A0 =C2=A0 =C2=A0} else if (vma->vm_ops && vma->= ;vm_ops->mremap) {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0err =3D vma->vm_op= s->mremap(new_vma, flags);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0err =3D vma->vm_op= s->mremap(new_vma);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (unlikely(err)) {
> --
> 2.31.0.rc2.261.g7f71774620-goog
>
>
--00000000000038cd4505be38542f--