[PATCH v4 07/16] mm: hugetlb: Fix Use-After-Free in unlock_or_release_subpool()

From: Ackerley Tng <[email protected]>

In unlock_or_release_subpool(), spool->lock was released before calling
subpool_is_free(spool). Because subpool_is_free() accesses spool->count and
spool->used_hpages locklessly, concurrent threads calling
hugepage_subpool_put_pages() or hugepage_put_subpool() could race when
spool->count is 0.

If Thread 1 drops spool->lock and is preempted before subpool_is_free(),
Thread 2 can acquire spool->lock, decrement spool->used_hpages to 0,
evaluate subpool_is_free() as true, and free spool via kfree(). When Thread
1 resumes, subpool_is_free() dereferences the freed spool pointer, causing
a Use-After-Free (UAF) or double free.

Fix this race by evaluating subpool_is_free() while holding spool->lock
prior to unlocking.

Fixes: 1d88433bb0085 ("mm/hugetlb: fix use after free when subpool max_hpages accounting is not enabled")
Cc: [email protected]
Signed-off-by: Ackerley Tng <[email protected]>
---
 mm/hugetlb.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 8431c00d48267..b759748468734 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -137,12 +137,11 @@ static inline bool subpool_is_free(struct hugepage_subpool *spool)
 static inline void unlock_or_release_subpool(struct hugepage_subpool *spool,
 						unsigned long irq_flags)
 {
+	bool is_free = subpool_is_free(spool);
+
 	spin_unlock_irqrestore(&spool->lock, irq_flags);
 
-	/* If no pages are used, and no other handles to the subpool
-	 * remain, give up any reservations based on minimum size and
-	 * free the subpool */
-	if (subpool_is_free(spool)) {
+	if (is_free) {
 		if (spool->min_hpages != -1)
 			hugetlb_acct_memory(spool->hstate,
 						-spool->min_hpages);

-- 
2.55.0.229.g6434b31f56-goog