mm/memfd: fix hugetlb reservation accounting in error paths

PATCH Superseded Hongfu Li 2 participants 4 messages Sep 3 – Sep 9

Subsystems (2)

Memory management tmpfs (shmem filesystem)

lore

Share & tools

lore

Hongfu Li

author

2026-09-03 03:01 UTC #0 lore raw

PATCH v2 1/1 mm/memfd: fix hugetlb reservation accounting in error paths

From: Hongfu Li <[email protected]>

If hugetlb_add_to_page_cache() in memfd_alloc_folio() fails with -EEXIST, a concurrent fault has already instantiated the folio in the page cache, and the reservation now belongs to that folio. Calling hugetlb_unreserve_pages() in that case incorrectly removes the region backing the cached folio. A later truncate or inode eviction then passes a negative (chg - freed) into hugepage_subpool_put_pages(), corrupting subpool and resv_huge_pages accounting.

Hold the hugetlb fault mutex from hugetlb_reserve_pages() until the error-path unreserve completes to make the reserve, allocate and instantiate steps atomic against concurrent faults. With the mutex held from the start, a concurrent fault can no longer consume the reservation between reserve and allocate/instantiate. If a fault completed before the mutex was taken, it has already added the region for that index, so hugetlb_reserve_pages() returns 0 and the error path leaves the region in place.

Fixes 717cf9357325 ("mm/memfd: reserve hugetlb folios before allocation")

Signed-off-by Hongfu Li <[email protected]>

---
v2:
- Take the hugetlb fault mutex before hugetlb_reserve_pages() and hold it until the error-path unreserve completes.
- Update commit message
- Link to v1: https://lore.kernel.org/all/[email protected]/

---
 mm/memfd.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

mm/memfd.c +16 ~~-15~~

index c708d92533f4..0f6fff004f5e 100644
@@ -82,22 +82,31 @@ struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx)
 		struct hstate *h = hstate_file(memfd);
 		int err = -ENOMEM;
 		long nr_resv;
+		u32 hash;
 
 		gfp_mask = htlb_alloc_mask(h);
 		gfp_mask &= ~(__GFP_HIGHMEM | __GFP_MOVABLE);
 		idx >>= huge_page_order(h);
 
+		/*
+		 * Serialize hugepage allocation and instantiation to prevent
+		 * races with concurrent allocations, as required by all other
+		 * callers of hugetlb_add_to_page_cache().
+		 */
+		hash = hugetlb_fault_mutex_hash(memfd->f_mapping, idx);
+		mutex_lock(&hugetlb_fault_mutex_table[hash]);
+
 		nr_resv = hugetlb_reserve_pages(inode, idx, idx + 1, NULL, EMPTY_VMA_FLAGS);
-		if (nr_resv < 0)
-			return ERR_PTR(nr_resv);
+		if (nr_resv < 0) {
+			err = nr_resv;
+			goto out_unlock;
+		}
 
 		folio = alloc_hugetlb_folio_reserve(h,
 						    numa_node_id(),
 						    NULL,
 						    gfp_mask);
 		if (folio) {
-			u32 hash;
-
 			/*
 			 * Zero the folio to prevent information leaks to userspace.
 			 * Use folio_zero_user() which is optimized for huge/gigantic

@@ -112,20 +121,9 @@ struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx)
 			 */
 			__folio_mark_uptodate(folio);
 
-			/*
-			 * Serialize hugepage allocation and instantiation to prevent
-			 * races with concurrent allocations, as required by all other
-			 * callers of hugetlb_add_to_page_cache().
-			 */
-			hash = hugetlb_fault_mutex_hash(memfd->f_mapping, idx);
-			mutex_lock(&hugetlb_fault_mutex_table[hash]);
-
 			err = hugetlb_add_to_page_cache(folio,
 							memfd->f_mapping,
 							idx);
-
-			mutex_unlock(&hugetlb_fault_mutex_table[hash]);
-
 			if (err) {
 				folio_put(folio);
 				goto err_unresv;

@@ -133,11 +131,14 @@ struct folio *memfd_alloc_folio(struct file *memfd, pgoff_t idx)
 
 			hugetlb_set_folio_subpool(folio, subpool_inode(inode));
 			folio_unlock(folio);
+			mutex_unlock(&hugetlb_fault_mutex_table[hash]);
 			return folio;
 		}
 err_unresv:
 		if (nr_resv > 0)
 			hugetlb_unreserve_pages(inode, idx, idx + 1, 0);
+out_unlock:
+		mutex_unlock(&hugetlb_fault_mutex_table[hash]);
 		return ERR_PTR(err);
 	}
 #endif

Signature · 1 line

2.54.0

Andrew Morton

maintainer

2026-09-03 20:22 UTC #1 lore raw

Quoted 9 lines · On Thu, 3 Sep 2026 11:01:34 +0800 Hongfu Li <[email protected]> wrote:

From: Hongfu Li <[email protected]>
If hugetlb_add_to_page_cache() in memfd_alloc_folio() fails with -EEXIST, a concurrent fault has already instantiated the folio in the page cache, and the reservation now belongs to that folio. Calling hugetlb_unreserve_pages() in that case incorrectly removes the region backing the cached folio. A later truncate or inode eviction then passes a negative (chg - freed) into hugepage_subpool_put_pages(), corrupting subpool and resv_huge_pages accounting.

Ho hum.

I've asked so many times "what are the userspace-visible runtime effects of this bug". Nowadays I often just ask Gemini instead. It told me:

Over time, these corrupted counters would leak huge page reservations. Applications using hugetlb memfds would eventually find themselves unable to allocate huge pages, receiving unexpected ENOMEM errors even though system memory and pool capacities appeared free and healthy.

and

The corrupted accounting caused hugepage_subpool_put_pages() to receive a negative value during a later file truncation or inode eviction.

While this typically manifests as kernel logs (WARN traces or badness flags regarding subpool page counts), it could cause misbehaved resource tracking that impacts subsequent system operations, unmounts, or process teardowns interacting with that hugetlb file descriptor.

All of which sounds rather unpleasant, so I suggest a cc:stable here.

To help people understand why we propose a backport and to help others understand the impact the fix will have upon their system, I'll paste the above into the changelog. Please send any necessary corrections.

Please also update your prompts (if using them) to ensure that the changelogging includes this info in the future.

I'll queue it for testing and shall await maintainer review.

Hongfu Li

author

2026-09-08 01:35 UTC #2 lore raw

Quoted 43 lines · Andrew Morton

Quoted 9 lines · On Thu, 3 Sep 2026 11:01:34 +0800 Hongfu Li <[email protected]> wrote:
From: Hongfu Li <[email protected]>
If hugetlb_add_to_page_cache() in memfd_alloc_folio() fails with -EEXIST, a concurrent fault has already instantiated the folio in the page cache, and the reservation now belongs to that folio. Calling hugetlb_unreserve_pages() in that case incorrectly removes the region backing the cached folio. A later truncate or inode eviction then passes a negative (chg - freed) into hugepage_subpool_put_pages(), corrupting subpool and resv_huge_pages accounting.
Ho hum.
I've asked so many times "what are the userspace-visible runtime effects of this bug". Nowadays I often just ask Gemini instead. It told me:
Over time, these corrupted counters would leak huge page reservations. Applications using hugetlb memfds would eventually find themselves unable to allocate huge pages, receiving unexpected ENOMEM errors even though system memory and pool capacities appeared free and healthy.
and
The corrupted accounting caused hugepage_subpool_put_pages() to receive a negative value during a later file truncation or inode eviction.
While this typically manifests as kernel logs (WARN traces or badness flags regarding subpool page counts), it could cause misbehaved resource tracking that impacts subsequent system operations, unmounts, or process teardowns interacting with that hugetlb file descriptor.
All of which sounds rather unpleasant, so I suggest a cc:stable here.
To help people understand why we propose a backport and to help others understand the impact the fix will have upon their system, I'll paste the above into the changelog. Please send any necessary corrections.

Quoted 5 lines

Please also update your prompts (if using them) to ensure that the changelogging includes this info in the future.
I'll queue it for testing and shall await maintainer review.

Thanks for the detailed explanation.

I've updated the changelog to include the userspace-visible effects and long-term impact as suggested.

Would you prefer that I send a v2 now, or should I wait for maintainer feedback first?

Signature · 2 lines

Best regards,
Hongfu

Andrew Morton

maintainer

2026-09-09 02:54 UTC #3 lore raw

Quoted 12 lines · On Tue, 8 Sep 2026 09:35:23 +0800 Hongfu Li <[email protected]> wrote:

Quoted 10 lines
To help people understand why we propose a backport and to help others understand the impact the fix will have upon their system, I'll paste the above into the changelog. Please send any necessary corrections.
Please also update your prompts (if using them) to ensure that the changelogging includes this info in the future.
I'll queue it for testing and shall await maintainer review.

Quoted 7 lines

Thanks for the detailed explanation.
I've updated the changelog to include the userspace-visible effects and long-term impact as suggested.
Would you prefer that I send a v2 now, or should I wait for maintainer feedback first?

Well, it's been five days. There have been 200,000 patches since then, so this series is at the far right end of the asymptote. Resend when convenient, please.