mm/hugetlb: restore reservation on error in hugetlb_mfill_atomic_pte() resubmission path

PATCH Superseded David Carlier 3 participants 6 messages Mar 22 – May 19

Subsystems (3)

Stable branch Memory management HugeTLB

lore

Share & tools

lore

David Carlier

author

2026-03-22 05:21 UTC #0 lore raw

PATCH 1/1 mm/hugetlb: restore reservation on error in hugetlb_mfill_atomic_pte() resubmission path

When the resubmission path in hugetlb_mfill_atomic_pte() allocates a new hugetlb folio via alloc_hugetlb_folio(), a VMA reservation is consumed. If copy_user_large_folio() subsequently fails, folio_put() restores the global hugetlb pool count through free_huge_folio(), but the per-VMA reservation map entry is left in an inconsistent state.

Add the missing restore_reserve_on_error() call before folio_put(), matching the first-attempt error path which already handles this correctly.

Fixes 8cc5fcbb5be8 ("mm, hugetlb: fix racy resv_huge_pages underflow on UFFDIO_COPY")

Cc [email protected]

Signed-off-by David Carlier <[email protected]>

---
 mm/hugetlb.c | 1 +
 1 file changed, 1 insertion(+)

mm/hugetlb.c +1 -0

index 88009cd2a846..d6ea11113f1d 100644
@@ -6295,6 +6295,7 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
 		folio_put(*foliop);
 		*foliop = NULL;
 		if (ret) {
+			restore_reserve_on_error(h, dst_vma, dst_addr, folio);
 			folio_put(folio);
 			goto out;
 		}

Signature · 1 line

2.53.0

Andrew Morton

maintainer

2026-03-23 19:13 UTC #1 lore raw

Quoted 14 lines · On Sun, 22 Mar 2026 05:21:20 +0000 David Carlier <[email protected]> wrote:

When the resubmission path in hugetlb_mfill_atomic_pte() allocates a new hugetlb folio via alloc_hugetlb_folio(), a VMA reservation is consumed. If copy_user_large_folio() subsequently fails, folio_put() restores the global hugetlb pool count through free_huge_folio(), but the per-VMA reservation map entry is left in an inconsistent state.
Add the missing restore_reserve_on_error() call before folio_put(), matching the first-attempt error path which already handles this correctly.
...

mm/hugetlb.c int hugetlb_mfill_atomic_pte(pte_t *dst_pte, · line 6301

Quoted 7 lines

mm/hugetlb.c +1 -0

@@ -6295,6 +6295,7 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
 		folio_put(*foliop);
 		*foliop = NULL;
 		if (ret) {
+			restore_reserve_on_error(h, dst_vma, dst_addr, folio);
 			folio_put(folio);
 			goto out;
 		}

I guess we could goto out_release_nounlock here, although I'm not sure that improves anything - keeping track of the value of folio_in_pagecache is rather twisty.

Andrew Morton

maintainer

2026-03-28 00:35 UTC #2 lore raw

In reply to David Carlier

Quoted 8 lines · On Sun, 22 Mar 2026 05:21:20 +0000 David Carlier <[email protected]> wrote:

When the resubmission path in hugetlb_mfill_atomic_pte() allocates a new hugetlb folio via alloc_hugetlb_folio(), a VMA reservation is consumed. If copy_user_large_folio() subsequently fails, folio_put() restores the global hugetlb pool count through free_huge_folio(), but the per-VMA reservation map entry is left in an inconsistent state.
Add the missing restore_reserve_on_error() call before folio_put(), matching the first-attempt error path which already handles this correctly.

Could we please have some review here?

Quoted 2 lines

Fixes 8cc5fcbb5be8 ("mm, hugetlb: fix racy resv_huge_pages underflow on UFFDIO_COPY")
Cc [email protected]

And agreement that this is desirable.

Thanks.

Quoted 15 lines

mm/hugetlb.c +1 -0

index 88009cd2a846..d6ea11113f1d 100644
@@ -6295,6 +6295,7 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
 		folio_put(*foliop);
 		*foliop = NULL;
 		if (ret) {
+			restore_reserve_on_error(h, dst_vma, dst_addr, folio);
 			folio_put(folio);
 			goto out;
 		}

Signature · 1 line

2.53.0

Muchun Song

maintainer

2026-04-04 13:00 UTC #3 lore raw

In reply to David Carlier

Quoted 10 lines · David Carlier

When the resubmission path in hugetlb_mfill_atomic_pte() allocates a new hugetlb folio via alloc_hugetlb_folio(), a VMA reservation is consumed. If copy_user_large_folio() subsequently fails, folio_put() restores the global hugetlb pool count through free_huge_folio(), but the per-VMA reservation map entry is left in an inconsistent state.

Quoted 4 lines

Add the missing restore_reserve_on_error() call before folio_put(), matching the first-attempt error path which already handles this correctly.
Fixes 8cc5fcbb5be8 ("mm, hugetlb: fix racy resv_huge_pages underflow on UFFDIO_COPY")

Hi David,

Thanks for this fix. The patch looks good to me and clearly solves the reservation leak in the resubmission path of hugetlb_mfill_atomic_pte().

However, I'm a bit curious about the Fixes tag. While commit 8cc5fcbb5be8 did introduce this code structure and the retry path, it seems the bug wasn't actually introduced there. At that time, copy_huge_page() returned void, so the failure path simply did not exist.

Instead, looking at the git history, the failure branch if (ret) was added later by commit 1cb9dc4b475c ("mm: hwpoison: support recovery from HugePage copy-on-write faults"). It modified copy_user_large_folio() to return an int and introduced error handling paths that unfortunately missed restoring the reservations. Should the Fixes tag perhaps point to 1cb9dc4b475c instead?

Furthermore, if commit 1cb9dc4b475c is indeed the root cause, I noticed it also introduced similar error handling paths in other places. For example, in copy_hugetlb_page_range():

        ret = copy_user_large_folio(new_folio, pte_folio, addr, dst_vma);
        folio_put(pte_folio);
        if (ret) {
                folio_put(new_folio);
                break;
        }

Here, new_folio was allocated with alloc_hugetlb_folio(), which consumes reservations. But if the copy fails, new_folio is freed via folio_put() without calling restore_reserve_on_error() first.

Does this imply we might have similar reservation leaks in other error paths touched by 1cb9dc4b475c? I'd love to hear your thoughts on this.

Thanks,
Muchun

Quoted 21 lines

Cc [email protected]

Signed-off-by David Carlier <[email protected]>

---
mm/hugetlb.c | 1 +

1 file changed, 1 insertion(+)

mm/hugetlb.c +1 -0

index 88009cd2a846..d6ea11113f1d 100644
@@ -6295,6 +6295,7 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_pte,
 	folio_put(*foliop);
 	*foliop = NULL;
 	if (ret) {
+ 		restore_reserve_on_error(h, dst_vma, dst_addr, folio);
 		folio_put(folio);
 		goto out;
 	}

Signature · 1 line

2.53.0

Andrew Morton

maintainer

2026-05-19 21:52 UTC #4 lore raw

Quoted 43 lines · On Sat, 4 Apr 2026 20:59:11 +0800 Muchun Song <[email protected]> wrote:

Quoted 10 lines · David Carlier
When the resubmission path in hugetlb_mfill_atomic_pte() allocates a new hugetlb folio via alloc_hugetlb_folio(), a VMA reservation is consumed. If copy_user_large_folio() subsequently fails, folio_put() restores the global hugetlb pool count through free_huge_folio(), but the per-VMA reservation map entry is left in an inconsistent state.
Quoted 4 lines
Add the missing restore_reserve_on_error() call before folio_put(), matching the first-attempt error path which already handles this correctly.
Fixes 8cc5fcbb5be8 ("mm, hugetlb: fix racy resv_huge_pages underflow on UFFDIO_COPY")
Hi David,
Thanks for this fix. The patch looks good to me and clearly solves the reservation leak in the resubmission path of hugetlb_mfill_atomic_pte().
However, I'm a bit curious about the Fixes tag. While commit 8cc5fcbb5be8 did introduce this code structure and the retry path, it seems the bug wasn't actually introduced there. At that time, copy_huge_page() returned void, so the failure path simply did not exist.
Instead, looking at the git history, the failure branch if (ret) was added later by commit 1cb9dc4b475c ("mm: hwpoison: support recovery from HugePage copy-on-write faults"). It modified copy_user_large_folio() to return an int and introduced error handling paths that unfortunately missed restoring the reservations. Should the Fixes tag perhaps point to 1cb9dc4b475c instead?
Furthermore, if commit 1cb9dc4b475c is indeed the root cause, I noticed it also introduced similar error handling paths in other places. For example, in copy_hugetlb_page_range():
        ret = copy_user_large_folio(new_folio, pte_folio, addr, dst_vma);
        folio_put(pte_folio);
        if (ret) {
                folio_put(new_folio);
                break;
        }

Quoted 6 lines

Here, new_folio was allocated with alloc_hugetlb_folio(), which consumes reservations. But if the copy fails, new_folio is freed via folio_put() without calling restore_reserve_on_error() first.
Does this imply we might have similar reservation leaks in other error paths touched by 1cb9dc4b475c? I'd love to hear your thoughts on this.

It's been a while. Can people please refocus on this?

David, your patch had cc:stable but I'm not seeing a description of the userspace-visible effects of the bug. Can you please describe?

From: David Carlier <[email protected]>
Subject: mm/hugetlb: restore reservation on error in hugetlb_mfill_atomic_pte() resubmission path
Date: Sun, 22 Mar 2026 05:21:20 +0000

Add the missing restore_reserve_on_error() call before folio_put(), matching the first-attempt error path which already handles this correctly.

Link https://lore.kernel.org/[email protected] open here

Fixes 8cc5fcbb5be8 ("mm, hugetlb: fix racy resv_huge_pages underflow on UFFDIO_COPY")

Signed-off-by David Carlier <[email protected]>

Cc David Hildenbrand <[email protected]>

Cc Mina Almasry <[email protected]>

Cc Muchun Song <[email protected]>

Cc Oscar Salvador <[email protected]>

Cc yuehaibing <[email protected]>

Cc <[email protected]>

Signed-off-by Andrew Morton <[email protected]>

---

 mm/hugetlb.c |    1 +
 1 file changed, 1 insertion(+)

mm/hugetlb.c renamed from mm/hugetlb.c~mm-hugetlb-restore-reservation-on-error-in-hugetlb_mfill_atomic_pte-resubmission-path +1 -0

@@ -6290,6 +6290,7 @@ int hugetlb_mfill_atomic_pte(pte_t *dst_
 		folio_put(*foliop);
 		*foliop = NULL;
 		if (ret) {
+			restore_reserve_on_error(h, dst_vma, dst_addr, folio);
 			folio_put(folio);
 			goto out;
 		}

David CARLIER

author

2026-05-19 22:41 UTC #5 lore raw

Quoted 2 lines · On Tue, 19 May 2026 14:52:15 -0700 Andrew Morton <[email protected]>…

David, your patch had cc:stable but I'm not seeing a description of the userspace-visible effects of the bug. Can you please describe?

When UFFDIO_COPY's resubmission copy_user_large_folio() fails (e.g. -EHWPOISON on a hwpoisoned source page), the per-VMA reservation map entry for dst_addr stays marked consumed while free_huge_folio() puts h->resv_huge_pages back. A later fault at the same address then takes the no-reservation path and, under pool pressure, SIGBUSes the task at an address it had already reserved. Each hit leaks one map entry for the lifetime of the mapping.

Quoted 1 line · On Sat, 4 Apr 2026 20:59:11 +0800 Muchun Song <[email protected]> wrote:

Should the Fixes tag perhaps point to 1cb9dc4b475c instead?

Yes. copy_user_large_folio() was void at 8cc5fcbb5be8, so the failing arm didn't exist yet; 1cb9dc4b475c is what made it int-returning and added the if (ret) { folio_put(folio); goto out; } without restore_reserve_on_error(). Will fix in v2.

Quoted 2 lines

Does this imply we might have similar reservation leaks in other error paths touched by 1cb9dc4b475c?

copy_hugetlb_page_range()'s hugetlb_try_dup_anon_rmap() fallback has the same shape and new_folio comes from alloc_hugetlb_folio(dst_vma, addr, false), so it leaks the same way. hugetlb_wp() is fine -- it goes through out_release_all which already calls restore_reserve_on_error(). I'll send the copy_hugetlb_page_range() fix as a separate patch.

Cheers.