[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <880b9e00-23d5-4c03-8269-8e2f9a50f358@amd.com>
Date: Tue, 8 Jul 2025 11:25:26 +0530
From: Shivank Garg <shivankg@....com>
To: Luiz Capitulino <luizcap@...hat.com>, david@...hat.com,
willy@...radead.org, akpm@...ux-foundation.org
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org, sj@...nel.org
Subject: Re: [PATCH v2 4/4] fs: stable_page_flags(): use snapshot_page()
On 7/8/2025 12:20 AM, Luiz Capitulino wrote:
> A race condition is possible in stable_page_flags() where user-space is
> reading /proc/kpageflags concurrently to a folio split. This may lead to
> oopses or BUG_ON()s being triggered.
>
> To fix this, this commit uses snapshot_page() in stable_page_flags() so
> that stable_page_flags() works with a stable page and folio snapshots
> instead.
>
> Note that stable_page_flags() makes use of some functions that require
> the original page or folio pointer to work properly (eg.
> is_free_budy_page() and folio_test_idle()). Since those functions can't
> be used on the page snapshot, we replace their usage with flags that
> were set by snapshot_page() for this purpose.
>
> Signed-off-by: Luiz Capitulino <luizcap@...hat.com>
> ---
> fs/proc/page.c | 29 +++++++++++++----------------
> 1 file changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/fs/proc/page.c b/fs/proc/page.c
> index 936f8bbe5a6f..cb58f066eb31 100644
> --- a/fs/proc/page.c
> +++ b/fs/proc/page.c
> @@ -147,6 +147,7 @@ static inline u64 kpf_copy_bit(u64 kflags, int ubit, int kbit)
> u64 stable_page_flags(const struct page *page)
> {
> const struct folio *folio;
> + struct page_snapshot ps;
> unsigned long k;
> unsigned long mapping;
> bool is_anon;
> @@ -158,7 +159,9 @@ u64 stable_page_flags(const struct page *page)
> */
> if (!page)
> return 1 << KPF_NOPAGE;
> - folio = page_folio(page);
> +
> + snapshot_page(&ps, page);
> + folio = &ps.folio_snapshot;
>
> k = folio->flags;
> mapping = (unsigned long)folio->mapping;
> @@ -167,7 +170,7 @@ u64 stable_page_flags(const struct page *page)
> /*
> * pseudo flags for the well known (anonymous) memory mapped pages
> */
> - if (page_mapped(page))
> + if (folio_mapped(folio))
> u |= 1 << KPF_MMAP;
> if (is_anon) {
> u |= 1 << KPF_ANON;
> @@ -179,7 +182,7 @@ u64 stable_page_flags(const struct page *page)
> * compound pages: export both head/tail info
> * they together define a compound page's start/end pos and order
> */
> - if (page == &folio->page)
> + if (ps.idx == 0)
> u |= kpf_copy_bit(k, KPF_COMPOUND_HEAD, PG_head);
> else
> u |= 1 << KPF_COMPOUND_TAIL;
> @@ -189,25 +192,19 @@ u64 stable_page_flags(const struct page *page)
> folio_test_large_rmappable(folio)) {
> /* Note: we indicate any THPs here, not just PMD-sized ones */
> u |= 1 << KPF_THP;
> - } else if (is_huge_zero_folio(folio)) {
> + } else if (is_huge_zero_pfn(ps.pfn)) {
> u |= 1 << KPF_ZERO_PAGE;
> u |= 1 << KPF_THP;
> - } else if (is_zero_folio(folio)) {
> + } else if (is_zero_pfn(ps.pfn)) {
> u |= 1 << KPF_ZERO_PAGE;
> }
>
> - /*
> - * Caveats on high order pages: PG_buddy and PG_slab will only be set
> - * on the head page.
> - */
> - if (PageBuddy(page))
> - u |= 1 << KPF_BUDDY;
> - else if (page_count(page) == 0 && is_free_buddy_page(page))
> + if (ps.flags & PAGE_SNAPSHOT_PG_FREE)
> u |= 1 << KPF_BUDDY;
>
> - if (PageOffline(page))
> + if (folio_test_offline(folio))
> u |= 1 << KPF_OFFLINE;
> - if (PageTable(page))
> + if (folio_test_pgtable(folio))
> u |= 1 << KPF_PGTABLE;
> if (folio_test_slab(folio))
> u |= 1 << KPF_SLAB;
> @@ -215,7 +212,7 @@ u64 stable_page_flags(const struct page *page)
> #if defined(CONFIG_PAGE_IDLE_FLAG) && defined(CONFIG_64BIT)
> u |= kpf_copy_bit(k, KPF_IDLE, PG_idle);
> #else
> - if (folio_test_idle(folio))
> + if (ps.flags & PAGE_SNAPSHOT_PG_IDLE)
> u |= 1 << KPF_IDLE;
> #endif
>
> @@ -241,7 +238,7 @@ u64 stable_page_flags(const struct page *page)
> if (u & (1 << KPF_HUGE))
> u |= kpf_copy_bit(k, KPF_HWPOISON, PG_hwpoison);
> else
> - u |= kpf_copy_bit(page->flags, KPF_HWPOISON, PG_hwpoison);
> + u |= kpf_copy_bit(ps.page_snapshot.flags, KPF_HWPOISON, PG_hwpoison);
> #endif
>
> u |= kpf_copy_bit(k, KPF_RESERVED, PG_reserved);
LGTM
Reviewed-by: Shivank Garg <shivankg@....com>
Powered by blists - more mailing lists