mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] hugetlb: retry private VMA locking when unmapping folios
@ 2026-10-15 17:05 Daehyeon Ko
  2026-10-20  8:47 ` David Hildenbrand (Arm)
  0 siblings, 1 reply; 4+ messages in thread
From: Daehyeon Ko @ 2026-10-15 17:05 UTC (permalink / raw)
  To: Muchun Song, Oscar Salvador; +Cc: David Hildenbrand, linux-mm, linux-kernel

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;
+			}
 			vma_lock = vma->vm_private_data;
 			/*
 			 * If we can not get vma lock, we need to drop

base-commit: 0fcb05b72344f40806cbe810de8872f1698bf053

^ permalink raw reply	[flat|nested] 4+ messages in thread

* 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

* Re: [PATCH] hugetlb: retry private VMA locking when unmapping folios
  2026-10-20  8:47 ` David Hildenbrand (Arm)
@ 2026-10-20 10:20   ` Daehyeon Ko
  2026-10-20 10:57     ` David Hildenbrand (Arm)
  0 siblings, 1 reply; 4+ messages in thread
From: Daehyeon Ko @ 2026-10-20 10:20 UTC (permalink / raw)
  To: david; +Cc: muchun.song, osalvador, linux-mm, linux-kernel

Hi David,

Yes, that makes sense.

The per-index fault mutex excludes concurrent faults for the folio being
removed, while i_mmap_rwsem keeps the VMA in the mapping tree and excludes
concurrent page-table teardown.  So the private resv_map lock is not needed
in this path.

I tested the exact diff from your message on v7.3-rc5 with KASAN+LOCKDEP.
My unprivileged mprotect()/hole-punch reproducer completed 1,000 rounds
(64,000 hole punches) with no KASAN, lockdep, unexpected warning, oops, or
panic.

Please consider my patch superseded by Josef's fix.

One small typo in the quoted comment: "i_mmap_rwsem everybody" appears to
be missing "excludes".

The quoted commit also predates the current linux-next policy change:
`Assisted-by` is no longer a recognized trailer.  `generated-content.rst`
asks that tool-assisted discovery be stated in the changelog prose instead.

Tested-by: Daehyeon Ko <4ncienth@gmail.com>

Thanks,
Daehyeon

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] hugetlb: retry private VMA locking when unmapping folios
  2026-10-20 10:20   ` Daehyeon Ko
@ 2026-10-20 10:57     ` David Hildenbrand (Arm)
  0 siblings, 0 replies; 4+ messages in thread
From: David Hildenbrand (Arm) @ 2026-10-20 10:57 UTC (permalink / raw)
  To: Daehyeon Ko; +Cc: muchun.song, osalvador, linux-mm, linux-kernel

On 10/20/26 12:20, Daehyeon Ko wrote:
> Hi David,
> 
> Yes, that makes sense.
> 
> The per-index fault mutex excludes concurrent faults for the folio being
> removed, while i_mmap_rwsem keeps the VMA in the mapping tree and excludes
> concurrent page-table teardown.  So the private resv_map lock is not needed
> in this path.
> 
> I tested the exact diff from your message on v7.3-rc5 with KASAN+LOCKDEP.
> My unprivileged mprotect()/hole-punch reproducer completed 1,000 rounds
> (64,000 hole punches) with no KASAN, lockdep, unexpected warning, oops, or
> panic.
> 
> Please consider my patch superseded by Josef's fix.
> 
> One small typo in the quoted comment: "i_mmap_rwsem everybody" appears to
> be missing "excludes".
> 
> The quoted commit also predates the current linux-next policy change:
> `Assisted-by` is no longer a recognized trailer.  `generated-content.rst`
> asks that tool-assisted discovery be stated in the changelog prose instead.
> 
> Tested-by: Daehyeon Ko <4ncienth@gmail.com>

Can you overhaul (shorten etc) the patch description and send it out for proper
review?

-- 
Cheers,

David

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-20 10:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2026-10-20 10:57     ` David Hildenbrand (Arm)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®