On Tue, May 12, 2026 at 10:42:14AM +0200, David Hildenbrand (Arm) wrote:
>
> > + for (; addr < end; addr += PAGE_SIZE) {
> > + vm_fault_t ret;
> > +
> > + ret = handle_mm_fault(vma, addr, fault_flags, NULL);
> > +
> > + if (ret & (VM_FAULT_RETRY | VM_FAULT_COMPLETED)) {
> > + /*
> > + * The mmap lock has been dropped by the fault handler.
> > + * Record the failing address and signal lock-drop to
> > + * the caller.
> > + */
> > + *hmm_vma_walk->locked = 0;
> > + hmm_vma_walk->last = addr;
> > + return -EAGAIN;
>
>
> Okay, so we'll return straight from hmm_vma_fault() to
> hmm_vma_handle_pte()/hmm_vma_walk_pmd() -> walk_page_range() machinery.
>
> Hopefully we don't refer to the MM/VMA on any path there? It would be nicer if
> the hmm_vma_fault() could be called by the caller of walk_page_range(), but
> that's tricky I guess, as hmm_vma_fault() consumes the walk structure and
> requires the vma in there.
>
It looks like a caller can provide a post_vma callback in mm_walk_ops. I
missed that case here. This callback cannot be supported by this change.
I will update the patch.
>
> Note: am I wrong, or is hmm_vma_fault() really always called with
> required_fault=true?
>
No, hmm_pte_need_fault can return false.
> > + }
> > +
> > + if (ret & VM_FAULT_ERROR)
> > return -EFAULT;
> > + }
> > return -EBUSY;
> > }
> >
> > @@ -566,6 +585,17 @@ static int hmm_vma_walk_hugetlb_entry(pte_t *pte,
> > unsigned long hmask,
> > if (required_fault) {
> > int ret;
> >
> > + /*
> > + * Faulting hugetlb pages on the unlockable path is not
> > + * supported. The walk framework holds hugetlb_vma_lock_read
> > + * which must be dropped before handle_mm_fault, but if the
> > + * mmap lock is also dropped (VM_FAULT_RETRY), the vma may
> > + * be freed and the walk framework's unconditional unlock
> > + * becomes a use-after-free.
> > + */
> > + if (hmm_vma_walk->locked)
> > + return -EFAULT;
>
> Just because it's unlockable doesn't mean that you must unlock. Can't this be
> kept working as is, just simulating here as if it would not be unlockable?
>
I’m not sure how to implement this. The walk_page_range code expects the
hugetlb VMA to still be read-locked when we return from
hmm_vma_walk_hugetlb_entry. How can we guarantee that if the VMA might
be gone?
I added a note in the docs. Whoever tackles this will likely need to
either rework `walk_page_range` to handle the case where the VMA is
gone, or use a different approach.
Do you have any other suggestions on how to implement it?
Thanks,
Stanislav
>
> --
> Cheers,
>
> David
