Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] hmm_range_fault_locked() performs VMA iteration on an external mm_struct without verifying if the address space is stable, risking a crash on an unstable maple tree. -- --- Patch [1]: [PATCH v2 1/4] fixup! mm/hmm: add hmm_range_fault_unlocked_timeout() for mmap lock-drop support --- commit a32e804e265d5dd30e11762ac51a889fc0abfebc Author: Stanislav Kinsburskii <[email protected]> fixup! mm/hmm: add hmm_range_fault_unlocked_timeout() for mmap lock-drop support This commit updates the example in the HMM documentation to pass the notifier stored in range to mmu_interval_read_retry(). It explicitly details the relationship between the HMM walk and the invalidation check. > diff --git a/Documentation/mm/hmm.rst b/Documentation/mm/hmm.rst > index 5c88d2cf08183..fc1b8dc198250 100644 > --- a/Documentation/mm/hmm.rst > +++ b/Documentation/mm/hmm.rst > @@ -206,7 +206,7 @@ The usage pattern is:: > goto out_put; > > take_lock(driver->update); > - if (mmu_interval_read_retry(&interval_sub, range.notifier_seq)) { > + if (mmu_interval_read_retry(range.notifier, range.notifier_seq)) { > release_lock(driver->update); > goto again; > } [ ... ] > @@ -225,7 +225,8 @@ The usage pattern is:: > The driver->update lock is the same lock that the driver takes inside its > invalidate() callback. That lock must be held before calling > mmu_interval_read_retry() to avoid any race with a concurrent CPU page table > -update. > +update. The retry check must use the same notifier and sequence number stored > +in ``range`` by ``hmm_range_fault_unlocked_timeout()``. > > Holding the mmap lock across HMM faults > ======================================= [Severity: High] This is a pre-existing issue, but while reviewing the documentation for HMM faults, I noticed hmm_range_fault_locked() in mm/hmm.c might be missing a check for a stable address space before traversing VMAs: mm/hmm.c:hmm_range_fault_locked() { ... mmap_assert_locked(mm); do { /* If range is no longer valid force retry. */ if (mmu_interval_check_retry(range->notifier, range->notifier_seq)) return -EBUSY; ret = walk_page_range(mm, hmm_vma_walk.last, range->end, &hmm_walk_ops, &hmm_vma_walk); ... } Does this path need to call check_stable_address_space(mm) before iterating the VMAs with walk_page_range()? If the mm becomes unstable (for instance, if an OOM reaper sets MMF_UNSTABLE and modifies the maple tree concurrently while a device driver processes a page fault for a user process), could iterating the maple tree on an external mm_struct encounter XA_ZERO_ENTRY markers and lead to undefined behavior or a panic? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/178406760622.1106335.2379450382728057793.stgit@skinsburskii?part=1
