linux-kernel - [PATCH] mm/folio: Avoid special handling for order value 0 in folio_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 15 Apr 2023 01:18:32 +0530
From:   Tarun Sahu <tsahu@...ux.ibm.com>
To:     linux-mm@...ck.org
Cc:     akpm@...ux-foundation.org, muchun.song@...ux.dev,
        mike.kravetz@...cle.com, aneesh.kumar@...ux.ibm.com,
        willy@...radead.org, sidhartha.kumar@...cle.com,
        gerald.schaefer@...ux.ibm.com, linux-kernel@...r.kernel.org,
        jaypatel@...ux.ibm.com, tsahu@...ux.ibm.com
Subject: [PATCH] mm/folio: Avoid special handling for order value 0 in folio_set_order

folio_set_order(folio, 0); which is an abuse of folio_set_order as 0-order
folio does not have any tail page to set order. folio->_folio_nr_pages is
set to 0 for order 0 in folio_set_order. It is required because
_folio_nr_pages overlapped with page->mapping and leaving it non zero
caused "bad page" error while freeing gigantic hugepages. This was fixed in
Commit ba9c1201beaa ("mm/hugetlb: clear compound_nr before freeing gigantic
pages"). Also commit a01f43901cfb ("hugetlb: be sure to free demoted CMA
pages to CMA") now explicitly clear page->mapping and hence we won't see
the bad page error even if _folio_nr_pages remains unset. Also the order 0
folios are not supposed to call folio_set_order, So now we can get rid of
folio_set_order(folio, 0) from hugetlb code path to clear the confusion.

The patch also moves _folio_set_head and folio_set_order calls in
__prep_compound_gigantic_folio() such that we avoid clearing them in the
error path.

Testing: I have run LTP tests, which all passes. and also I have written
the test in LTP which tests the bug caused by compound_nr and page->mapping
overlapping.

https://lore.kernel.org/all/20230413090753.883953-1-tsahu@linux.ibm.com/

Running on older kernel ( < 5.10-rc7) with the above bug this fails while
on newer kernel and, also with this patch it passes.

Signed-off-by: Tarun Sahu <tsahu@...ux.ibm.com>
---
 mm/hugetlb.c  | 9 +++------
 mm/internal.h | 8 ++------
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index f16b25b1a6b9..e2540269c1dc 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1489,7 +1489,6 @@ static void __destroy_compound_gigantic_folio(struct folio *folio,
 			set_page_refcounted(p);
 	}

-	folio_set_order(folio, 0);
 	__folio_clear_head(folio);
 }

@@ -1951,9 +1950,6 @@ static bool __prep_compound_gigantic_folio(struct folio *folio,
 	struct page *p;

 	__folio_clear_reserved(folio);
-	__folio_set_head(folio);
-	/* we rely on prep_new_hugetlb_folio to set the destructor */
-	folio_set_order(folio, order);
 	for (i = 0; i < nr_pages; i++) {
 		p = folio_page(folio, i);

@@ -1999,6 +1995,9 @@ static bool __prep_compound_gigantic_folio(struct folio *folio,
 		if (i != 0)
 			set_compound_head(p, &folio->page);
 	}
+	__folio_set_head(folio);
+	/* we rely on prep_new_hugetlb_folio to set the destructor */
+	folio_set_order(folio, order);
 	atomic_set(&folio->_entire_mapcount, -1);
 	atomic_set(&folio->_nr_pages_mapped, 0);
 	atomic_set(&folio->_pincount, 0);
@@ -2017,8 +2016,6 @@ static bool __prep_compound_gigantic_folio(struct folio *folio,
 		p = folio_page(folio, j);
 		__ClearPageReserved(p);
 	}
-	folio_set_order(folio, 0);
-	__folio_clear_head(folio);
 	return false;
 }

diff --git a/mm/internal.h b/mm/internal.h
index 18cda26b8a92..0d96a3bc1d58 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -425,16 +425,12 @@ int split_free_page(struct page *free_page,
  */
 static inline void folio_set_order(struct folio *folio, unsigned int order)
 {
-	if (WARN_ON_ONCE(!folio_test_large(folio)))
+	if (WARN_ON_ONCE(!order || !folio_test_large(folio)))
 		return;

 	folio->_folio_order = order;
 #ifdef CONFIG_64BIT
-	/*
-	 * When hugetlb dissolves a folio, we need to clear the tail
-	 * page, rather than setting nr_pages to 1.
-	 */
-	folio->_folio_nr_pages = order ? 1U << order : 0;
+	folio->_folio_nr_pages = 1U << order;
 #endif
 }

-- 
2.31.1