* Re: [PATCH] hugetlb: retry private VMA locking when unmapping folios
2026-10-15 17:05 [PATCH] hugetlb: retry private VMA locking when unmapping folios Daehyeon Ko
@ 2026-10-20 8:47 ` David Hildenbrand (Arm)
2026-10-20 10:20 ` Daehyeon Ko
0 siblings, 1 reply; 4+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-20 8:47 UTC (permalink / raw)
To: Daehyeon Ko, Muchun Song, Oscar Salvador; +Cc: linux-mm, linux-kernel
On 10/15/26 19:05, Daehyeon Ko wrote:
> hugetlb_unmap_file_folio() takes a reference to the hugetlb_vma_lock in
> vm_private_data when its VMA trylock fails. It then drops i_mmap_rwsem and
> waits for the lock in the established order.
>
> That fallback is only valid for VM_MAYSHARE VMAs. Since commit
> bf4916922c60 ("hugetlbfs: extend hugetlb_vma_lock to private VMAs"), the
> trylock can also fail for a private owner VMA. Its vm_private_data contains
> resv_map | HPAGE_RESV_OWNER, not a hugetlb_vma_lock. The fallback then
> performs a misaligned refcount operation and treats reservation-map list
> fields as an rwsem.
>
> Private reservation maps have no stable VMA backpointer. Drop i_mmap_rwsem
> and restart the interval-tree walk when their trylock fails. Keep the
> referenced fallback for shareable VMAs.
>
> On a system with a pre-provisioned hugepage pool, an unprivileged
> reproducer racing mprotect() with hole punching triggered the refcount
> warning followed by a general protection fault in rwsem_down_write_slowpath
> on v7.3-rc5 and 6.12.111. With this change, a v7.3-rc5 KASAN+LOCKDEP build
> completed 1,000 rounds and 64,000 hole punches without a warning, lockdep
> report, oops, or panic. The reproducer is available privately on request.
>
> Fixes: bf4916922c60 ("hugetlbfs: extend hugetlb_vma_lock to private VMAs")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
> ---
> fs/hugetlbfs/inode.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index 7611a8470ea265..9a676228e2070a 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -402,6 +402,17 @@ static void hugetlb_unmap_file_folio(struct hstate *h,
> continue;
>
> if (!hugetlb_vma_trylock_write(vma)) {
> + /*
> + * The referenced fallback below is specific to the
> + * hugetlb_vma_lock stored by shareable VMAs. A private
> + * owner VMA stores a tagged resv_map pointer here instead.
> + */
> + if (!(vma->vm_flags & VM_MAYSHARE)) {
> + i_mmap_unlock_write(mapping);
> + cond_resched();
> + i_mmap_lock_write(mapping);
> + goto retry;
That looks like we can just live-lock now?
> + }
> vma_lock = vma->vm_private_data;
This whole function is a mess.
I have the following AI fix in a queue-of-fixes to go through and send out.
The description is a bit mouthful, but the fix itself looks much cleaner (just skip the
write-lock for VMAs that don't need it):
Does that make sense?
commit 12a35d8ee3e638129080c2bb13c0a35c873b5257
Author: Josef Bacik <josef@toxicpanda.com>
Date: Thu Oct 8 06:55:26 2026 +0000
hugetlbfs: fix unmapping a folio from a private vma
hugetlb_unmap_file_folio() unmaps a folio which is about to leave the
page cache from every vma that maps it. Where it can not get the lock of
a vma with hugetlb_vma_trylock_write(), it takes vma->vm_private_data for
a struct hugetlb_vma_lock, gets a reference to it, drops i_mmap_rwsem and
sleeps on its rw_sema.
Since commit bf4916922c60 ("hugetlbfs: extend hugetlb_vma_lock to private
VMAs") the trylock can fail for a MAP_PRIVATE vma that owns a reservation
map, too. The lock of such a vma is resv_map->rw_sema, which every fault
in the vma holds for reading, and its vm_private_data is the address of
the resv_map with HPAGE_RESV_OWNER in bit 0. kref_get() and down_write()
then work on the bytes of the resv_map, one byte off.
A private vma maps a folio of the page cache after a read fault. Three
threads of an unprivileged process are enough, on a hugetlb memfd: one
reads a page through a private mapping, one writes to another page of
that mapping, one allocates the first page with fallocate() and punches
it out again. ftruncate() and MADV_REMOVE do as well as the hole punch.
refcount_t: addition on 0; use-after-free.
WARNING: lib/refcount.c:25 at refcount_warn_saturate+0x6a/0x90
remove_inode_hugepages+0x49d/0x530
hugetlbfs_fallocate+0x4bf/0x5f0
vfs_fallocate+0x13d/0x330
On x86-64, saturating the "reference count" stores 0xc0 into the first
byte of resv_map->lock: the next fault in the vma spins in region_chg()
for ever. down_write() either takes a list pointer, shifted by one byte,
for the owner and dies of a general protection fault, or spins with
interrupts off on what it takes for the wait_lock, in both cases with the
fault mutex and the inode lock held.
Leave the lock of a vma that is not shareable alone, as it was before that
commit. That lock keeps faults out of the vma until MADV_DONTNEED has
freed its pages. It is not needed here: the caller holds the fault mutex
of the index, so nobody can map the folio or replace it by a copy, and
i_mmap_rwsem, held for writing, keeps the vma in the file's tree and its
page tables in place.
Fixes: bf4916922c60 ("hugetlbfs: extend hugetlb_vma_lock to private VMAs")
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index d21dbcdc705ed..02cec281ceba5 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -401,6 +401,19 @@ static void hugetlb_unmap_file_folio(struct hstate *h,
if (!hugetlb_vma_maps_pfn(vma, v_start, pfn))
continue;
+ /*
+ * Only a shareable vma has a struct hugetlb_vma_lock. The
+ * lock of a private vma lives in its resv_map and keeps
+ * faults away from MADV_DONTNEED; it is not needed here:
+ * the fault mutex holds off faults on this index, and
+ * i_mmap_rwsem everybody who zaps or frees page tables.
+ */
+ if (!__vma_shareable_lock(vma)) {
+ unmap_hugepage_range(vma, v_start, v_end, NULL,
+ ZAP_FLAG_DROP_MARKER);
+ continue;
+ }
+
if (!hugetlb_vma_trylock_write(vma)) {
vma_lock = vma->vm_private_data;
/*
--
Cheers,
David
^ permalink raw reply [flat|nested] 4+ messages in thread