On Tue, May 26, 2026 at 12:57 AM Wei Yang <[email protected]> wrote: > > On Mon, May 25, 2026 at 12:10:41PM -0700, Andrew Morton wrote: > >On Mon, 25 May 2026 08:15:53 -0600 Nico Pache <[email protected]> wrote: > > > >> Can you please append the following fixup that reverts one of the > >> changes requested in V17. The issue with the change is described > >> below. > > > >OK. fyi, what I received was badly mangled: wordwrapping, tabs messed > >up, etc. > > > >Here's my reconstruction: > > > > Hi, Nico > > I tried to reply your mail, but found it has some encoding problem, so reply > here.
Yeah sorry I didnt properly configure my email client after getting a new laptop. > > > > >Author: Nico Pache <[email protected]> > >Subject: fix potential use-after-free of vma in mthp_collapse() > >Date: Mon May 25 07:38:59 2026 -0600 > > > >Between V17 and v18, one reviewer (Wei) brought up that we are not doing > >the uffd-armed check until deep in the collapse operation. While not > >functionally incorrect, it can lead to unnecessary work. > > So we decide to tolerate the behavioral change? Yes, I believe it is ok for now. Either way we needed to remove the potential UAF. It only affects the behavior if mTHP is enabled, so the legacy behavior is kept. And the uffd case is limited. My future work involves further optimizing and cleaning up khugepaged. I'll make this part of the goal too. My first thought is to do the revalidation at every order (between the locks dropping); but that essentially pays the same penalty... I can't think of a clean solution at the moment. Does that sound ok? Cheers, -- Nico -- Nico > > > > >We optimized this by passing the vma variable to mthp_collapse() and using > >the collapse_max_ptes_none() function to check the state of uffd-armed > >preventing the wasted work later in the collapse. > > > >mthp_collapse() is called after mmap_read_unlock(), so the vma pointer can > >become stale. Remove the vma parameter and pass NULL to > >collapse_max_ptes_none() instead. > > > >Link: https://lore.kernel.org/[email protected] > >Signed-off-by: Nico Pache <[email protected]> > >... > > > > mm/khugepaged.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > >--- a/mm/khugepaged.c~mm-khugepaged-introduce-mthp-collapse-support-fix > >+++ a/mm/khugepaged.c > >@@ -1502,9 +1502,9 @@ static unsigned int collapse_mthp_count_ > > * If a collapse is permitted, we attempt to collapse the PTE range into a > > * mTHP. > > */ > >-static int mthp_collapse(struct mm_struct *mm, struct vm_area_struct *vma, > >- unsigned long address, int referenced, int unmapped, > >- struct collapse_control *cc, unsigned long enabled_orders) > >+static int mthp_collapse(struct mm_struct *mm, unsigned long address, > >+ int referenced, int unmapped, struct collapse_control *cc, > >+ unsigned long enabled_orders) > > { > > unsigned int nr_occupied_ptes, nr_ptes, max_ptes_none; > > int collapsed = 0, stack_size = 0; > >@@ -1524,7 +1524,7 @@ static int mthp_collapse(struct mm_struc > > if (!test_bit(order, &enabled_orders)) > > goto next_order; > > > >- max_ptes_none = collapse_max_ptes_none(cc, vma, order); > >+ max_ptes_none = collapse_max_ptes_none(cc, NULL, order); > > > > nr_occupied_ptes = collapse_mthp_count_present(cc, offset, > > nr_ptes); > >@@ -1749,7 +1749,7 @@ out_unmap: > > if (result == SCAN_SUCCEED) { > > /* collapse_huge_page expects the lock to be dropped before > > calling */ > > mmap_read_unlock(mm); > >- nr_collapsed = mthp_collapse(mm, vma, start_addr, referenced, > >+ nr_collapsed = mthp_collapse(mm, start_addr, referenced, > > unmapped, cc, enabled_orders); > > /* mmap_lock was released above, set lock_dropped */ > > *lock_dropped = true; > >_ > > -- > Wei Yang > Help you, Help me >
