linux-kernel - Re: [PATCH net-next v6 9/9] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is

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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <577ff7bd-3f30-4531-a7f4-5c63da797848@suse.cz>
Date: Mon, 23 Jun 2025 20:09:30 +0200
From: Vlastimil Babka <vbabka@...e.cz>
To: Mina Almasry <almasrymina@...gle.com>,
 Pavel Begunkov <asml.silence@...il.com>
Cc: David Hildenbrand <david@...hat.com>, Zi Yan <ziy@...dia.com>,
 Byungchul Park <byungchul@...com>, willy@...radead.org,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org, linux-mm@...ck.org,
 kernel_team@...ynix.com, kuba@...nel.org, ilias.apalodimas@...aro.org,
 harry.yoo@...cle.com, hawk@...nel.org, akpm@...ux-foundation.org,
 davem@...emloft.net, john.fastabend@...il.com, andrew+netdev@...n.ch,
 toke@...hat.com, tariqt@...dia.com, edumazet@...gle.com, pabeni@...hat.com,
 saeedm@...dia.com, leon@...nel.org, ast@...nel.org, daniel@...earbox.net,
 lorenzo.stoakes@...cle.com, Liam.Howlett@...cle.com, rppt@...nel.org,
 surenb@...gle.com, mhocko@...e.com, horms@...nel.org,
 linux-rdma@...r.kernel.org, bpf@...r.kernel.org, vishal.moola@...il.com,
 hannes@...xchg.org, jackmanb@...gle.com
Subject: Re: [PATCH net-next v6 9/9] page_pool: access ->pp_magic through
 struct netmem_desc in page_pool_page_is_pp()

On 6/23/25 19:28, Mina Almasry wrote:
> On Mon, Jun 23, 2025 at 10:05 AM Pavel Begunkov <asml.silence@...il.com> wrote:
>>
>>
>> As you said, it's just a sanity check, all page pool pages should
>> be freed by the networking code. It checks the ownership with
>> netmem_is_pp(), which is basically the same as page_pool_page_is_pp()
>> but done though some aliasing.
>>
>> static inline bool netmem_is_pp(netmem_ref netmem)
>> {
>>         return (netmem_get_pp_magic(netmem) & PP_MAGIC_MASK) == PP_SIGNATURE;
>> }
>>
>> I assume there is no point in moving the check to skbuff.c as it
>> already does exactly same test, but we can probably just kill it.
>>
> 
> Even if we do kill it, maybe lets do that in a separate patch, and
> maybe a separate series. I would recommend not complicating this one?
> 
> Also, AFAIU, this is about removing/moving the checks in
> bad_page_reason() and page_expected_state()? I think this check does
> fire sometimes. I saw at least 1 report in the last year of a
> bad_page_reason() check firing because the page_pool got its
> accounting wrong and released a page to the buddy allocator early, so
> maybe that new patch that removes that check should explain why this
> check is no longer necessary.

Note these checks are these days only done with CONFIG_DEBUG_VM, or
debugging/hardening options like debug_pagealloc/init_on_alloc/init_on_free,
see mem_debugging_and_hardening_init() and when check_pages_enabled static
key is enabled.