Re: [PATCH] mm/memory: fix hugetlb_zap_begin() call in zap_vma_range_batched()

[Andrew Morton]

On Thu,  3 Sep 2026 17:00:26 -0700 SJ Park <sj@xxxxxxxxxx> wrote:

> Commit f1fc44daf618 ("mm/hugetlb: don't lock private resv_map during
> final unmap") added zap_details parameter to hugetlb_zap_begin().  But
> the hugetlb_zap_begin() call in zap_vma_range_batched() is not updated.
> As a result, build fails as below.  Fix it.
> 
>   CC      mm/memory.o
> .../mm/memory.c: In function ‘zap_vma_range_batched’:
> .../mm/memory.c:2308:9: error: too few arguments to function ‘hugetlb_zap_begin’
>  2308 |         hugetlb_zap_begin(vma, &range.start, &range.end);
>       |         ^~~~~~~~~~~~~~~~~
> In file included from .../mm/memory.c:48:
> .../include/linux/hugetlb.h:253:20: note: declared here
>   253 | static inline void hugetlb_zap_begin(struct vm_area_struct *vma,
>       |                    ^~~~~~~~~~~~~~~~~

You cleverly pulled during the ten-minute-window after I'd pushed this
out in order to pull it onto my build-test-machine.

There's probably a smarter way of doing this, not sure what though.

It doesn't happen often - I usually only need to push/pull the quilt
patches (25-new).

> /* TODO: move below to commentary */
> 
> I didn't read the broken commit in depth.  This fix is only
> build-tested.  I wanted to report the issue with this as a temporal fix,
> but the broken commit doesn't have Link: tag.  So directly posting this
> temporal and not very well verified fix first.

Yeah, this is possible fix for
https://syzkaller.appspot.com/bug?extid=bd6aaf99e8443d8a9034 which I
had chatgpt create for me.  It's in limbo at present until I figure out
what to do with it.  Actually I'll hide it from others while figuring-out
happens.



For the morbidly curious.  It's really only a 2-line change, plus a bunch
of changes to pass the zap_details down to  __hugetlb_zap_begin().



From: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Subject: mm/hugetlb: don't lock private resv_map during final unmap

Replacing a private hugetlb mapping can trigger a lockdep circular
locking warning and, if the corresponding reclaim, NBD and socket paths
run concurrently, can deadlock userspace tasks.

The mmap path holds mmap_lock for write while removing an overlapping
mapping and then reaches:

  unmap_vmas()
    hugetlb_zap_begin()
      hugetlb_vma_lock_write()
        resv_map->rw_sema

This establishes the lock ordering:

  mmap_lock -> resv_map->rw_sema

Lockdep already knows about a transitive dependency in the other
direction.  In full, the relevant part of the dependency graph is:

  resv_map->rw_sema
    -> fs_reclaim
    -> q->q_usage_counter
    -> q->elevator_lock
    -> set->srcu
    -> cmd->lock
    -> nsock->tx_lock
    -> sk_lock-AF_INET6
    -> mmap_lock

The resv_map->rw_sema -> fs_reclaim edge can be established by a
private hugetlb fault.  The fault holds the private VMA lock for read
and huge_pte_alloc() can allocate page-table memory with reclaim
enabled.  The middle of the chain comes from the block and NBD paths,
while sk_lock-AF_INET6 -> mmap_lock can be established when an IPv6
send copies from userspace while holding the socket lock and faults on
the user buffer.

Consequently, lockdep summarizes the relevant reverse path as:

  resv_map->rw_sema -> sk_lock-AF_INET6 -> mmap_lock

This is a transitive lockdep dependency, not a single call stack
holding all three locks.

Commit bf4916922c60 ("hugetlbfs: extend hugetlb_vma_lock to private
VMAs") made hugetlb_vma_lock_write() acquire resv_map->rw_sema for
private hugetlb mappings.  That lock is needed for partial zaps such as
MADV_DONTNEED.  It keeps a concurrent fault from running after the PTE
has been cleared but before the hugepage has actually been returned to
the pool, which could otherwise result in an unexpected SIGBUS when the
hugepage pool is fully allocated.

That serialization is unnecessary when the VMA is being finally
unmapped.  mmap_lock prevents a concurrent fault from entering a VMA
which is being removed, and private VMAs do not participate in hugetlb
PMD sharing.

Pass the zap details to hugetlb_zap_begin() so that it can distinguish
a final unmap.  For final unmaps, continue taking the hugetlb VMA lock
for shareable mappings, where it protects PMD sharing and the lifetime
of the VMA lock, but do not take resv_map->rw_sema for a private
mapping.  Likewise, do not attempt to release the private reservation
map lock from hugetlb_zap_end().

Non-final zaps continue taking resv_map->rw_sema, preserving the
MADV_DONTNEED versus page-fault serialization for which private hugetlb
VMA locking was introduced.

Fixes: bf4916922c60 ("hugetlbfs: extend hugetlb_vma_lock to private VMAs")
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Reported-by: syzbot+bd6aaf99e8443d8a9034@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=bd6aaf99e8443d8a9034
Cc: Rik van Riel <riel@xxxxxxxxxxx>
Cc: Muchun Song <muchun.song@xxxxxxxxx>
Cc: Oscar Salvador <osalvador@xxxxxxx>
Cc: David Hildenbrand <david@xxxxxxxxxx>
Cc: Liam R. Howlett <liam@xxxxxxxxxxxxx>
Cc: Lorenzo Stoakes <ljs@xxxxxxxxxx>
Cc: Michal Hocko <mhocko@xxxxxxxx>
Cc: Mike Rapoport <rppt@xxxxxxxxxx>
Cc: Suren Baghdasaryan <surenb@xxxxxxxxxx>
Cc: Vlastimil Babka <vbabka@xxxxxxxxxx>
Cc: Jane Chu <jane.chu@xxxxxxxxxx>
Assisted-by: ChatGPT <chatgpt@xxxxxxxxxx>
Cc: <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---

 include/linux/hugetlb.h |    8 +++++---
 mm/hugetlb.c            |   16 ++++++++++++++--
 mm/memory.c             |    4 ++--
 3 files changed, 21 insertions(+), 7 deletions(-)

--- a/include/linux/hugetlb.h~mm-hugetlb-dont-lock-private-resv_map-during-final-unmap
+++ a/include/linux/hugetlb.h
@@ -245,15 +245,17 @@ void adjust_range_if_pmd_sharing_possibl
 				unsigned long *start, unsigned long *end);
 
 extern void __hugetlb_zap_begin(struct vm_area_struct *vma,
-				unsigned long *begin, unsigned long *end);
+				unsigned long *begin, unsigned long *end,
+				struct zap_details *details);
 extern void __hugetlb_zap_end(struct vm_area_struct *vma,
 			      struct zap_details *details);
 
 static inline void hugetlb_zap_begin(struct vm_area_struct *vma,
-				     unsigned long *start, unsigned long *end)
+				     unsigned long *start, unsigned long *end,
+				     struct zap_details *details)
 {
 	if (is_vm_hugetlb_page(vma))
-		__hugetlb_zap_begin(vma, start, end);
+		__hugetlb_zap_begin(vma, start, end, details);
 }
 
 static inline void hugetlb_zap_end(struct vm_area_struct *vma,
--- a/mm/hugetlb.c~mm-hugetlb-dont-lock-private-resv_map-during-final-unmap
+++ a/mm/hugetlb.c
@@ -5403,13 +5403,25 @@ void __unmap_hugepage_range(struct mmu_g
 }
 
 void __hugetlb_zap_begin(struct vm_area_struct *vma,
-			 unsigned long *start, unsigned long *end)
+			 unsigned long *start, unsigned long *end,
+			 struct zap_details *details)
 {
+	zap_flags_t zap_flags = details ? details->zap_flags : 0;
+
 	if (!vma->vm_file)	/* hugetlbfs_file_mmap error */
 		return;
 
 	adjust_range_if_pmd_sharing_possible(vma, start, end);
-	hugetlb_vma_lock_write(vma);
+
+	/*
+	 * A final unmap cannot race with a fault in this VMA because
+	 * mmap_lock prevents the fault from entering a VMA which is being
+	 * removed.  Skip the private resv_map lock in that case to avoid
+	 * inverting its lock order with mmap_lock.  Shareable mappings
+	 * still need the VMA lock to protect PMD sharing.
+	 */
+	if (!(zap_flags & ZAP_FLAG_UNMAP) || __vma_shareable_lock(vma))
+		hugetlb_vma_lock_write(vma);
 	if (vma->vm_file)
 		i_mmap_lock_write(vma->vm_file->f_mapping);
 }
--- a/mm/memory.c~mm-hugetlb-dont-lock-private-resv_map-during-final-unmap
+++ a/mm/memory.c
@@ -2268,7 +2268,7 @@ void unmap_vmas(struct mmu_gather *tlb,
 		unsigned long start = max(vma->vm_start, unmap->vma_start);
 		unsigned long end = min(vma->vm_end, unmap->vma_end);
 
-		hugetlb_zap_begin(vma, &start, &end);
+		hugetlb_zap_begin(vma, &start, &end, &details);
 		__zap_vma_range(tlb, vma, start, end, &details);
 		hugetlb_zap_end(vma, &details);
 		vma = mas_find(unmap->mas, unmap->tree_end - 1);
@@ -2304,7 +2304,7 @@ void zap_vma_range_batched(struct mmu_ga
 
 	mmu_notifier_range_init(&range, MMU_NOTIFY_CLEAR, 0, vma->vm_mm,
 				address, end);
-	hugetlb_zap_begin(vma, &range.start, &range.end);
+	hugetlb_zap_begin(vma, &range.start, &range.end, details);
 	update_hiwater_rss(vma->vm_mm);
 	mmu_notifier_invalidate_range_start(&range);
 	/*
_