Re: [PATCH v3 03/40] mm/vma: introduce and use vma_[flags_]can_merge()

Re: [PATCH v3 03/40] mm/vma: introduce and use vma_[flags_]can_merge()

[Gregory Price]

On Thu, Sep 17, 2026 at 05:22:12PM +0100, Lorenzo Stoakes (ARM) wrote:
> Replace the open-coded VMA_SPECIAL_FLAGS check in the VMA merge logic with
> two new functions vma_flags_can_merge() and vma_can_merge() and update the
> merge logic to use the former.
> 
> This abstracts the check and expresses it in terms of the desired behaviour
> rather than an arbitrary and confusing VMA flag.
> 
> This also lays the groundwork for making further improvements in VMA flag
> usage.
> 
> Also update the userland VMA tests to reflect the change.
> 
> No functional change intended.
> 
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

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

Re: [PATCH v3 04/40] mm: consistently validate VMA state after mmap[_prepare] hooks

[Gregory Price]

On Thu, Sep 17, 2026 at 05:22:13PM +0100, Lorenzo Stoakes (ARM) wrote:
>  static inline int mmap_file(struct file *file, struct vm_area_struct *vma)
>  {
...
> +	err = mmap_hook_validate(prev_start, prev_end, &prev_flags, vma);
> +	if (unlikely(err)) {
> +		vma->vm_start = prev_start;
> +		vma->vm_end = prev_end;
> +		vma_close(vma);
>  	}
> +
> +	return err;
>  }
>  

I indepdeantly validated the sashiko report on this chunk.  Seems like
close() should be deferred until after __map_new_file_vma() calls
unmap_region().

suggested fix is to drop vma_close() from mmap_file()  and update the
cleanup in __mmap_new_file_vma() 

if (error) {
	UNMAP_STATE(unmap, vmi, vma, vma->vm_start, vma->vm_end,
		    map->prev, map->next);
	vma_iter_set(vmi, vma->vm_end);
	unmap_region(&unmap);

	/* Release driver state only after its mappings are gone. */
	vma_close(vma);

	if (map_same_file(map))
		fput(map->vm_file);
	vma->vm_file = NULL;

	return error;
}

Example race:

  Thread A                              Thread B

  mmap(MAP_FIXED, address A)
    driver remap_pfn_range(A, page P)
                                         load/store at known address A
                                         hardware finds the new present PTE
    validation fails
    ->close() frees page P
                                         UAF
    unmap_region()
    TLB shootdown

With that fix

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

~Gregory

Re: [PATCH v3 04/40] mm: consistently validate VMA state after mmap[_prepare] hooks

[Lorenzo Stoakes (ARM)]

On Thu, Sep 24, 2026 at 01:17:28PM -0400, Gregory Price wrote:
> On Thu, Sep 17, 2026 at 05:22:13PM +0100, Lorenzo Stoakes (ARM) wrote:
> >  static inline int mmap_file(struct file *file, struct vm_area_struct *vma)
> >  {
> ...
> > +	err = mmap_hook_validate(prev_start, prev_end, &prev_flags, vma);
> > +	if (unlikely(err)) {
> > +		vma->vm_start = prev_start;
> > +		vma->vm_end = prev_end;
> > +		vma_close(vma);
> >  	}
> > +
> > +	return err;
> >  }
> >
>
> I indepdeantly validated the sashiko report on this chunk.  Seems like
> close() should be deferred until after __map_new_file_vma() calls
> unmap_region().

Ack perhaps too quickly dismissed that one...!

>
> suggested fix is to drop vma_close() from mmap_file()  and update the
> cleanup in __mmap_new_file_vma()
>
> if (error) {
> 	UNMAP_STATE(unmap, vmi, vma, vma->vm_start, vma->vm_end,
> 		    map->prev, map->next);
> 	vma_iter_set(vmi, vma->vm_end);
> 	unmap_region(&unmap);
>
> 	/* Release driver state only after its mappings are gone. */
> 	vma_close(vma);
>
> 	if (map_same_file(map))
> 		fput(map->vm_file);
> 	vma->vm_file = NULL;
>
> 	return error;
> }
>
> Example race:
>
>   Thread A                              Thread B
>
>   mmap(MAP_FIXED, address A)
>     driver remap_pfn_range(A, page P)
>                                          load/store at known address A
>                                          hardware finds the new present PTE
>     validation fails
>     ->close() frees page P
>                                          UAF
>     unmap_region()
>     TLB shootdown
>
> With that fix

Ack, yeah. It's kind of a situation that should never happen, but if validation
is supposed to actually be run against things then we should keep the kernel
stable when we do it :)

Will apply for the respin.

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

Thanks!

>
> ~Gregory
>

--
Cheers, Lorenzo

Re: [PATCH v3 05/40] mm/vma: ensure mmap_prepare doesn't set actions on a mergeable vma

[Gregory Price]

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

otherwise

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

Re: [PATCH v3 05/40] mm/vma: ensure mmap_prepare doesn't set actions on a mergeable vma

[Lorenzo Stoakes (ARM)]

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

Re: [PATCH v3 06/40] mm: make map_kernel_pages_[prepare,complete] internal and unexported

[Gregory Price]

On Thu, Sep 17, 2026 at 05:22:15PM +0100, Lorenzo Stoakes (ARM) wrote:
> There's no reason to export the symbols for these functions which are only
> called from internal mm logic, additionally there's no reason for them to
> be declared in mm.h.
> 
> This patch therefore removes the exports and moves the declarations to
> mm/internal.h.
> 
> No functional change intended.
> 
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

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

Re: [PATCH v3 07/40] mm/vma: tidy up map kernel pages enum values

[Gregory Price]

On Thu, Sep 17, 2026 at 05:22:16PM +0100, Lorenzo Stoakes (ARM) wrote:
> MMAP_MAP_KERNEL_PAGES is a mouthful, discard the MAP_ as that's implied by
> MMAP.
> 
> Also while we're here delete useless comments for mmap actions whose names
> clearly indicate what they are for.
> 
> Also update the userland VMA tests to reflect this change.
> 
> No functional change intended.
> 
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>

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