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) <[email protected]>
Thanks!
>
> ~Gregory
>
--
Cheers, Lorenzo
