[net,V2] page_pool: mask the page->signature before the checking

Commit Message

Yunsheng Lin Aug. 6, 2021, 1:39 a.m. UTC

As mentioned in commit c07aea3ef4d4 ("mm: add a signature in
struct page"):
"The page->signature field is aliased to page->lru.next and
page->compound_head."

And as the comment in page_is_pfmemalloc():
"lru.next has bit 1 set if the page is allocated from the
pfmemalloc reserves. Callers may simply overwrite it if they
do not need to preserve that information."

The page->signature is OR’ed with PP_SIGNATURE when a page is
allocated in page pool, see __page_pool_alloc_pages_slow(),
and page->signature is checked directly with PP_SIGNATURE in
page_pool_return_skb_page(), which might cause resoure leaking
problem for a page from page pool if bit 1 of lru.next is set
for a pfmemalloc page. What happens here is that the original
pp->signature is OR'ed with PP_SIGNATURE after the allocation
in order to preserve any existing bits(such as the bit 1, used
to indicate a pfmemalloc page), so when those bits are present,
those page is not considered to be from page pool and the DMA
mapping of those pages will be left stale.

As bit 0 is for page->compound_head, So mask both bit 0/1 before
the checking in page_pool_return_skb_page(). And we will return
those pfmemalloc pages back to the page allocator after cleaning
up the DMA mapping.

Fixes: 6a5bcd84e886 ("page_pool: Allow drivers to hint on SKB recycling")
Reviewed-by: Ilias Apalodimas <[e-mail]>
Signed-off-by: Yunsheng Lin <[e-mail]>
---
V2: explain more on why we need to mask those bits.
---
 net/core/page_pool.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

[e-mail] Aug. 9, 2021, 9:10 a.m. UTC | #1

Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Fri, 6 Aug 2021 09:39:07 +0800 you wrote:
> As mentioned in commit c07aea3ef4d4 ("mm: add a signature in
> struct page"):
> "The page->signature field is aliased to page->lru.next and
> page->compound_head."
> 
> And as the comment in page_is_pfmemalloc():
> "lru.next has bit 1 set if the page is allocated from the
> pfmemalloc reserves. Callers may simply overwrite it if they
> do not need to preserve that information."
> 
> [...]

Here is the summary with links:
  - [net,V2] page_pool: mask the page->signature before the checking
    https://git.kernel.org/netdev/net/c/0fa32ca438b4

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Patch

diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 5e4eb45..8ab7b40 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -634,7 +634,15 @@  bool page_pool_return_skb_page(struct page *page)
 	struct page_pool *pp;
 
 	page = compound_head(page);
-	if (unlikely(page->pp_magic != PP_SIGNATURE))
+
+	/* page->pp_magic is OR'ed with PP_SIGNATURE after the allocation
+	 * in order to preserve any existing bits, such as bit 0 for the
+	 * head page of compound page and bit 1 for pfmemalloc page, so
+	 * mask those bits for freeing side when doing below checking,
+	 * and page_is_pfmemalloc() is checked in __page_pool_put_page()
+	 * to avoid recycling the pfmemalloc page.
+	 */
+	if (unlikely((page->pp_magic & ~0x3UL) != PP_SIGNATURE))
 		return false;
 
 	pp = page->pp;