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

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

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