Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Gregory Price <gourry@gourry.net>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	 linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-usb@vger.kernel.org,  linux-rdma@vger.kernel.org,
	selinux@vger.kernel.org, linux-sound@vger.kernel.org,
	 bpf@vger.kernel.org, linux-scsi@vger.kernel.org,
	linux-fbdev@vger.kernel.org,  dri-devel@lists.freedesktop.org,
	linux-trace-kernel@vger.kernel.org,
	 linux-perf-users@vger.kernel.org, linux-arch@vger.kernel.org,
	linux-fsdevel@vger.kernel.org,
	 linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev,
	linuxppc-dev@lists.ozlabs.org,  kvm@vger.kernel.org,
	kvm-riscv@lists.infradead.org, linux-riscv@lists.infradead.org,
	 linux-s390@vger.kernel.org, sparclinux@vger.kernel.org,
	fuse-devel@lists.linux.dev
Subject: Re: [PATCH v3 05/40] mm/vma: ensure mmap_prepare doesn't set actions on a mergeable vma
Date: Fri, 25 Sep 2026 10:51:04 +0100	[thread overview]
Message-ID: <arZC7UscjpjVJxnV@gremlin> (raw)
In-Reply-To: <arVkwC_wNoTLjRYf@gourry-fedora-PF4VCD3F>

On Thu, Sep 24, 2026 at 02:00:18PM -0400, Gregory Price wrote:
> On Thu, Sep 17, 2026 at 05:22:14PM +0100, Lorenzo Stoakes (ARM) wrote:
> > When a user requests an mmap_action be performed in mmap_prepare, this
> > involves populating the VMA range with data.
> >
> > However, if the VMA is mergeable, it might then mistakenly be merged with
> > another VMA without having populated the range.
> >
> > Every mmap action currently available sets VMA flags such that the VMA
> > cannot be merged.
> >
> > However, to ensure that no future mmap action falls foul of this, assert
> > that this is the case upon mmap_prepare validation.
> >
> > Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
> > ---
> >  mm/vma.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/mm/vma.c b/mm/vma.c
> > index d6ed10cefc8f..62f2ce1ad5a1 100644
> > --- a/mm/vma.c
> > +++ b/mm/vma.c
> > @@ -2809,6 +2809,15 @@ static int mmap_validate(unsigned long prev_start, unsigned long prev_end,
> >  int mmap_prepare_validate(const struct vm_area_desc *prev_desc,
> >  			  const struct vm_area_desc *desc)
> >  {
> > +	/*
> > +	 * It is not valid to execute mmap actions for VMAs which can be merged,
> > +	 * as any such merge would leave portions of the mapping incorrectly
> > +	 * unmapped.
> > +	 */
> > +	if (vma_flags_can_merge(&desc->vma_flags) &&
> > +	    WARN_ON_ONCE(desc->action.type != MMAP_NOTHING))
> > +		return -EINVAL;
> > +
>
> If you wanted to make this unit-testable, you could pull it out into a
> separate function:
>
> static bool mmap_action_is_valid(const struct vm_area_desc *desc)
> {
> 	return desc->action.type == MMAP_NOTHING ||
> 		!vma_flags_can_merge(&desc->vma_flags);
> }
>
> then write:
>
> if (WARN_ON_ONCE(!mmap_action_is_valid(desc)))
> 	return -EINVAL;
>
> And you can write a unit test directly against mmap_action_is_valid

You mean to isolate this check specifically?

All of the functions in vma.c are unit-testable in the userland VMA tests,
obviously here you'd be testing further stuff but you could certainly assert a
mergeable VMA specifying an action should result in an error there.

I'm also keen not to proliferate two many 'kinds' of validation.

As mmap_validate() checks pretty much everything BUT the action check, and it
has to work across mmap_prepare and mmap hooks.

So the idea here is we put the mmap_prepare-specific stuff in
mmap_prepare_validate() and the shared stuff in mmap_validate().

And already the stuff that can be validated just against flags lives in
mmap_validate_vma_flags() so that is itself separated out nicely.

>
> otherwise
>
> Reviewed-by: Gregory Price (Meta) <gourry@gourry.net>

Thanks!

--
Cheers, Lorenzo


  reply	other threads:[~2026-09-25 11:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260917-b4-mmap-prepare-vma-flag-sanify-v3-0-4583d8a23bca@kernel.org>
     [not found] ` <20260917-b4-mmap-prepare-vma-flag-sanify-v3-3-4583d8a23bca@kernel.org>
2026-09-24 16:38   ` [PATCH v3 03/40] mm/vma: introduce and use vma_[flags_]can_merge() Gregory Price
     [not found] ` <20260917-b4-mmap-prepare-vma-flag-sanify-v3-4-4583d8a23bca@kernel.org>
2026-09-24 17:17   ` [PATCH v3 04/40] mm: consistently validate VMA state after mmap[_prepare] hooks Gregory Price
2026-09-25 12:51     ` Lorenzo Stoakes (ARM)
     [not found] ` <20260917-b4-mmap-prepare-vma-flag-sanify-v3-5-4583d8a23bca@kernel.org>
2026-09-24 18:00   ` [PATCH v3 05/40] mm/vma: ensure mmap_prepare doesn't set actions on a mergeable vma Gregory Price
2026-09-25  9:51     ` Lorenzo Stoakes (ARM) [this message]
     [not found] ` <20260917-b4-mmap-prepare-vma-flag-sanify-v3-6-4583d8a23bca@kernel.org>
2026-09-29 15:58   ` [PATCH v3 06/40] mm: make map_kernel_pages_[prepare,complete] internal and unexported Gregory Price
     [not found] ` <20260917-b4-mmap-prepare-vma-flag-sanify-v3-7-4583d8a23bca@kernel.org>
2026-09-29 15:59   ` [PATCH v3 07/40] mm/vma: tidy up map kernel pages enum values Gregory Price

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=arZC7UscjpjVJxnV@gremlin \
    --to=ljs@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=bpf@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=fuse-devel@lists.linux.dev \
    --cc=gourry@gourry.net \
    --cc=kvm-riscv@lists.infradead.org \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.linux.dev \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=selinux@vger.kernel.org \
    --cc=sparclinux@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox