[PATCH v2 0/4] Fix HugeTLB subpool used_hpages tracking

Ackerley Tng via B4 Relay posted 4 patches 2 weeks, 1 day ago
There is a newer version of this series
Documentation/mm/hugetlbfs_reserv.rst              |  17 +--
.../translations/zh_CN/mm/hugetlbfs_reserv.rst     |  11 +-
fs/hugetlbfs/inode.c                               |   8 +-
include/linux/hugetlb.h                            |   4 +-
mm/hugetlb.c                                       | 147 +++++++++++----------
5 files changed, 90 insertions(+), 97 deletions(-)
[PATCH v2 0/4] Fix HugeTLB subpool used_hpages tracking
Posted by Ackerley Tng via B4 Relay 2 weeks, 1 day ago
HugeTLB subpools currently only track used_hpages when the user configures
a size limit.

This is buggy since when there are existing allocations from the subpool
that would have satisfied the minimum reservations,
hugepage_subpool_put_pages() will still restore a reservation to the
subpool. See below for an example of a false reservation.

In addition, the subpool is considered free prematurely, is freed, and this
ends up causing a use-after-free.

The fix is to always track used_hpages within subpools, which is also
beneficial in general because with that information, reservation tracking
is also fully managed within hugepage_subpool_put_pages().

The invariant is that now (if min_hpages is requested),

  used_hpages + rsv_hpages >= min_hpages

This allows hugepage_subpool_put_pages() to always be able to report how
many reservations it can absorb and hence return an accurate number of
reservations to be returned to the global pool.

Hugepage reservations and restorations can always happen in parallel, so
relying on local variables to compute whether to restore to the global pool
(on allocation failure) is not safe.

There is complexity and some bugs in hugetlb_reserve_pages() and
alloc_hugetlb_folio() failure handling paths.

+ In hugetlb_reserve_pages(): On hugetlb_acct_memory() failure,
  out_put_pages manually calculates how many pages to return using local
  variables, which is race-prone and can leak reservations or underflow
  global counters.
+ In alloc_hugetlb_folio(): When allocation fails and gbl_chg == 1,
  out_subpool_put skips hugepage_subpool_put_pages(), permanently leaking
  used_hpages.

By tracking used_hpages in the subpool, these allocation/reservation paths
can handle failures by consistently returning pages to the subpool, and
relying on the return value to restore reservations to the global pool.

This series changes subpools to always track used_hpages, which itself
fixes the false reservation bug, and then uses used_hpages tracking in
subpools to fix other bugs.

This series is a subset of patches from [1] and replaces [1].

[1] https://lore.kernel.org/all/20260722-hugetlb-alloc-failure-fixes-v4-0-88e8b81970dc@google.com/

Tested:

+ Reproducers (see below) pass
+ tools/testing/selftests/mm/ksft_hugetlb.sh passes
+ libhugetlbfs tests pass

Changes in v2:

+ Add patch 4 to avoid a possible page allocation in the cleanup path of
  hugetlb_reserve_pages(), addresses Sashiko's comment on v1.

v1: https://lore.kernel.org/r/20260902-hugetlb-subpool-always-track-used-v1-0-de1cd14bd713@google.com

I have reproducers, get them from

https://github.com/googleprodkernel/linux-cc/commits/hugetlb-subpool-always-track-used-with-reproducers-v2

Here's an example of a false restoration:

1. Mount time
    + spool->min_hpages = 1 (user requested min_size=2M)
    + spool->max_hpages = -1 (no maximum size specified)
    + spool->rsv_hpages = 1 (reserve min_hpages)
    + spool->used_hpages = 0 (not tracked when max_hpages == -1)
    + h->resv_huge_pages = 1 (reserved by hugetlb_acct_memory(h, 1))
2. Shared mapping of 4MB (2 pages) created (mmap with MAP_SHARED)
    + In hugetlb_reserve_pages(), region_chg() finds chg = 2 (pages 0 and 1
      need reservations)
    + hugepage_subpool_get_pages(spool, 2)
        + spool->rsv_hpages = 0 (consumed the 1 subpool reservation)
        + Return 1, since this subpool only had 1 reservation
    + hugetlb_acct_memory(h, 1)
        + h->resv_huge_pages = 2 (incremented from 1 to 2 for the global
          reservation)
        + region_add() records reservations for pages 0 and 1 in the inode
          resv_map
3. Process touches and populates Page 0:
    + hugetlb_no_page() calls alloc_hugetlb_folio()
    + Page 0 reuses the existing reservation (vma_needs_reservation()
      returns 0 => map_chg = MAP_CHG_REUSE = 0)
    + hugepage_subpool_get_pages() is not called (map_chg == 0)
    + dequeue_hugetlb_folio_nodemask() consumes 1 reservation:
      h->resv_huge_pages--;
    + h->resv_huge_pages = 1 (decremented from 2 to 1)
4. Process closes the file and exits:
    + For MAP_SHARED mappings, reservations persist in the inode resv_map
    + Page 1 reservation remains active
    + h->resv_huge_pages = 1 (retained for Page 1)
5. File is truncated to 2MB (truncate -s 2M):
    + Truncation invokes remove_inode_hugepages() for page range [1,
      LONG_MAX)
    + Page 1 was never faulted into page cache => freed = 0
    + Calls hugetlb_unreserve_pages(inode, 1, LONG_MAX, freed = 0)
    + region_del() removes Page 1 from resv_map => chg = 1
6. Inside hugetlb_unreserve_pages(): hugepage_subpool_put_pages(1)
    + delta = chg - freed = 1 - 0 = 1
    + Because spool->max_hpages == -1, spool->used_hpages always = 0
    + spool->used_hpages < spool->min_hpages => 0 < 1 => true
        <<== subpool assumes 0 pages are in use, ignoring allocated Page 0
    + spool->rsv_hpages + delta <= spool->min_hpages => 0 + 1 <= 1 => true
    + spool->rsv_hpages += 1 => spool->rsv_hpages = 1
        <<== false reservation restored to subpool!
    + Return 0 (subpool absorbed the reservation)
7. Back in hugetlb_unreserve_pages(): hugetlb_acct_memory()
    + hugetlb_acct_memory(h, -0) => does nothing
    + h->resv_huge_pages = 1 (remains 1, not decremented)
    + Both reservations have ended (Page 0 allocated, Page 1 truncated),
      but h->resv_huge_pages remains stuck at 1
8. Later during unmounting:
    + subpool_is_free() checks spool->rsv_hpages == spool->min_hpages => 1
      == 1 => true
    + Because spool->rsv_hpages was falsely restored to 1, the subpool is
      erroneously considered completely free
    + hugetlb_acct_memory(spool->hstate, -spool->min_hpages) decrements
      h->resv_huge_pages by 1 (1 - 1 = 0), masking the leak on unmount
9. If the folio outlives the inode, when the folio is freed (Page 0),
   free_huge_folio() will read subpool from the folio and act on it =>
   use-after-free and then double free

Signed-off-by: Ackerley Tng <ackerleytng@google.com>
---
Ackerley Tng (4):
      mm: hugetlb: Track used_hpages when getting/putting pages from subpool
      mm: hugetlb: Fix out_put_pages subpool reserve calculation
      mm: hugetlb: Fix subpool usage leak on allocation failure
      mm: hugetlb: Avoid re-allocating global reservations on region add failure

 Documentation/mm/hugetlbfs_reserv.rst              |  17 +--
 .../translations/zh_CN/mm/hugetlbfs_reserv.rst     |  11 +-
 fs/hugetlbfs/inode.c                               |   8 +-
 include/linux/hugetlb.h                            |   4 +-
 mm/hugetlb.c                                       | 147 +++++++++++----------
 5 files changed, 90 insertions(+), 97 deletions(-)
---
base-commit: df2908090cda368b01ff43709f51890076c56157
change-id: 20260902-hugetlb-subpool-always-track-used-2624840f4c08

Best regards,
--
Ackerley Tng <ackerleytng@google.com>
Re: [PATCH v2 0/4] Fix HugeTLB subpool used_hpages tracking
Posted by Andrew Morton 2 weeks, 1 day ago
On Wed, 09 Sep 2026 14:49:25 -0700 Ackerley Tng via B4 Relay <devnull+ackerleytng.google.com@kernel.org> wrote:

> HugeTLB subpools currently only track used_hpages when the user configures
> a size limit.
> 
> This is buggy since when there are existing allocations from the subpool
> that would have satisfied the minimum reservations,
> hugepage_subpool_put_pages() will still restore a reservation to the
> subpool. See below for an example of a false reservation.

That sounds annoying, although isn't clear how this affects end-users.

> In addition, the subpool is considered free prematurely, is freed, and this
> ends up causing a use-after-free.

That sounds alarming.

Do you think it's best for us to submit [1-3] for -stable backporting?

Is it feasible to come up with a set of small little fixes to get
-stable out of trouble and then to prepare broader updates for our
ongoing mainline development?

If it's "shut up Andrew you're always saying that" then OK, I can take
that :)
Re: [PATCH v2 0/4] Fix HugeTLB subpool used_hpages tracking
Posted by Ackerley Tng 1 week, 4 days ago
Andrew Morton <akpm@linux-foundation.org> writes:

> On Wed, 09 Sep 2026 14:49:25 -0700 Ackerley Tng via B4 Relay <devnull+ackerleytng.google.com@kernel.org> wrote:
>
>> HugeTLB subpools currently only track used_hpages when the user configures
>> a size limit.
>>
>> This is buggy since when there are existing allocations from the subpool
>> that would have satisfied the minimum reservations,
>> hugepage_subpool_put_pages() will still restore a reservation to the
>> subpool. See below for an example of a false reservation.

"false reservation" should have read "false reservation
restoration". Will fix in the next revision.

>
> That sounds annoying, although isn't clear how this affects end-users.
>

I'll also add this to next revision:

hugepage_subpool_put_pages() falsely absorbing reservations (aka
reservations being falsely restored to the subpool) means
hugepage_subpool_put_pages() will return 0, and h->resv_huge_pages won't
be decremented. The page(s) still being reserved in h->resv_huge_pages
means fewer HugeTLB page(s) for use in the entire host.

>> In addition, the subpool is considered free prematurely, is freed, and this
>> ends up causing a use-after-free.
>
> That sounds alarming.
>
> Do you think it's best for us to submit [1-3] for -stable backporting?
>
> Is it feasible to come up with a set of small little fixes to get
> -stable out of trouble and then to prepare broader updates for our
> ongoing mainline development?

Please see [1]. I don't think it'll be easy to come up with a complete
set of small fixes to fix all the issues that are fixed here.

>
> If it's "shut up Andrew you're always saying that" then OK, I can take
> that :)

[1] https://lore.kernel.org/all/CAEvNRgHbzY30n1hz2vv7BiBMMhci+NEv_ubP1LjQNmBuu6Kjgw@mail.gmail.com/
Re: [PATCH v2 0/4] Fix HugeTLB subpool used_hpages tracking
Posted by Ackerley Tng 1 week, 4 days ago
Ackerley Tng <ackerleytng@google.com> writes:

> Andrew Morton <akpm@linux-foundation.org> writes:
>
>> On Wed, 09 Sep 2026 14:49:25 -0700 Ackerley Tng via B4 Relay <devnull+ackerleytng.google.com@kernel.org> wrote:
>>
>>> HugeTLB subpools currently only track used_hpages when the user configures
>>> a size limit.
>>>
>>> This is buggy since when there are existing allocations from the subpool
>>> that would have satisfied the minimum reservations,
>>> hugepage_subpool_put_pages() will still restore a reservation to the
>>> subpool. See below for an example of a false reservation.
>
> "false reservation" should have read "false reservation
> restoration". Will fix in the next revision.
>
>>
>> That sounds annoying, although isn't clear how this affects end-users.
>>
>
> I'll also add this to next revision:
>
> hugepage_subpool_put_pages() falsely absorbing reservations (aka
> reservations being falsely restored to the subpool) means
> hugepage_subpool_put_pages() will return 0, and h->resv_huge_pages won't
> be decremented. The page(s) still being reserved in h->resv_huge_pages
> means fewer HugeTLB page(s) for use in the entire host.
>
>>> In addition, the subpool is considered free prematurely, is freed, and this
>>> ends up causing a use-after-free.
>>
>> That sounds alarming.
>>
>> Do you think it's best for us to submit [1-3] for -stable backporting?
>>
>> Is it feasible to come up with a set of small little fixes to get
>> -stable out of trouble and then to prepare broader updates for our
>> ongoing mainline development?
>
> Please see [1]. I don't think it'll be easy to come up with a complete
> set of small fixes to fix all the issues that are fixed here.
>

I'm hoping this will make it to 7.4 and it's 7.3-rc3 now, so I'll respin
perhaps tomorrow or so if people are ok with the suggestions on the 4
patches or if there are no objections!

>>
>> If it's "shut up Andrew you're always saying that" then OK, I can take
>> that :)
>
> [1] https://lore.kernel.org/all/CAEvNRgHbzY30n1hz2vv7BiBMMhci+NEv_ubP1LjQNmBuu6Kjgw@mail.gmail.com/