netdev - [PATCH net-next] mm/page_pool: catch page

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]

Message-ID: <170082101266.1085481.12199867179160710331.stgit@firesoul>
Date: Fri, 24 Nov 2023 11:16:52 +0100
From: Jesper Dangaard Brouer <hawk@...nel.org>
To: netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>, vbabka@...e.cz
Cc: Jesper Dangaard Brouer <hawk@...nel.org>, bpf@...r.kernel.org,
 Eric Dumazet <eric.dumazet@...il.com>,
 "David S. Miller" <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>,
 linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
 Mel Gorman <mgorman@...hsingularity.net>,
 Matthew Wilcox <willy@...radead.org>, kernel-team@...udflare.com
Subject: [PATCH net-next] mm/page_pool: catch page_pool memory leaks

Pages belonging to a page_pool (PP) instance must be freed through the
PP APIs in-order to correctly release any DMA mappings and release
refcnt on the DMA device when freeing PP instance. When PP release a
page (page_pool_release_page) the page->pp_magic value is cleared.

This patch detect a leaked PP page in free_page_is_bad() via
unexpected state of page->pp_magic value being PP_SIGNATURE.

We choose to report and treat it as a bad page. It would be possible
to release the page via returning it to the PP instance as the
page->pp pointer is likely still valid.

Notice this code is only activated when either compiled with
CONFIG_DEBUG_VM or boot cmdline debug_pagealloc=on, and
CONFIG_PAGE_POOL.

Reduced example output of leak with PP_SIGNATURE = dead000000000040:

 BUG: Bad page state in process swapper/4  pfn:141fa6
 page:000000006dbf8062 refcount:0 mapcount:0 mapping:0000000000000000 index:0x141fa6000 pfn:0x141fa6
 flags: 0x2fffff80000000(node=0|zone=2|lastcpupid=0x1fffff)
 page_type: 0xffffffff()
 raw: 002fffff80000000 dead000000000040 ffff88814888a000 0000000000000000
 raw: 0000000141fa6000 0000000000000001 00000000ffffffff 0000000000000000
 page dumped because: page_pool leak
 [...]
 Call Trace:
  <IRQ>
  dump_stack_lvl+0x32/0x50
  bad_page+0x70/0xf0
  free_unref_page_prepare+0x263/0x430
  free_unref_page+0x34/0x130
  mlx5e_free_rx_mpwqe+0x190/0x1c0 [mlx5_core]
  mlx5e_post_rx_mpwqes+0x1ac/0x280 [mlx5_core]
  mlx5e_napi_poll+0x12b/0x710 [mlx5_core]
  ? skb_free_head+0x4f/0x90
  __napi_poll+0x2b/0x1c0
  net_rx_action+0x27b/0x360

The advantage is the Call Trace directly points to the function
leaking the PP page, which in this case is an on purpose bug
introduced into the mlx5 driver to test this code change.

Currently PP will periodically in page_pool_release_retry()
printk warning "stalled pool shutdown" which cannot be directly
corrolated to leaking and might as well be a false positive
due to SKBs being stuck on a socket for an extended period.
After this patch we should be able to remove this printk.

Signed-off-by: Jesper Dangaard Brouer <hawk@...nel.org>
---
 mm/page_alloc.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 733732e7e0ba..37ca4f4b62bf 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -915,6 +915,9 @@ static inline bool page_expected_state(struct page *page,
 			page_ref_count(page) |
 #ifdef CONFIG_MEMCG
 			page->memcg_data |
+#endif
+#ifdef CONFIG_PAGE_POOL
+			((page->pp_magic & ~0x3UL) == PP_SIGNATURE) |
 #endif
 			(page->flags & check_flags)))
 		return false;
@@ -941,6 +944,10 @@ static const char *page_bad_reason(struct page *page, unsigned long flags)
 #ifdef CONFIG_MEMCG
 	if (unlikely(page->memcg_data))
 		bad_reason = "page still charged to cgroup";
+#endif
+#ifdef CONFIG_PAGE_POOL
+	if (unlikely((page->pp_magic & ~0x3UL) == PP_SIGNATURE))
+		bad_reason = "page_pool leak";
 #endif
 	return bad_reason;
 }