Re: [PATCH v4 net] net: page_pool: fix UAF in __page_pool_release_netmem_dma on xa_cmpxchg race

on 2026/8/5 11:50, Mina Almasry wrote:
> On Tue, Aug 4, 2026 at 7:16 PM Jakub Kicinski <[email protected]> wrote:
>> On Fri, 31 Jul 2026 10:37:58 -0700 Mina Almasry wrote:
>>>> +       __page_pool_unmap_netmem_dma(pool, netmem);
>>>>          page_pool_set_dma_addr_netmem(netmem, 0);
>>>> +       if (likely(PP_DMA_INDEX_BITS))
>>>> +               netmem_set_dma_index(netmem, 0);
>>> I now notice that maybe another cleanup we could have done is open
>>> code __page_pool_unmap_netmem_dma() in this function to cut down 1
>>> helper, and just have the scrub function call
>>> __page_pool_release_netmem_dma() to reduce some code. But this is more
>>> than fine too I think, especially since this is a fix the stable trees
>>> are going to want I guess.
>> Not sure this is a good idea? scrub is trying to touch just the DMA
>> mapping, right? It shouldn't try to update the page itself because
>> it has no reference to the page, the page may get freed in parallel.
>> Hopefully DMA unmap on a freed page is legal..
> Scrub actually frees the page in the loop in
> page_pool_empty_ring(pool), so it's not true AFAIU that it 'shouldn't
> try to update the page'.
>
> My mental model (roughly) is that the pp has a ref and that single
> last ref can be dropped in the page_pool_destroy() path or the
> page_pool_put_netmem() path, but the ref can't be dropped twice and
> the same goes for dma unmapping.
>
> Now that i look closer at the code I have no idea why we didn't 'just'
> put the dma-unmapping inside of page_pool_return_netmem(). That code
> path is already common between scrub() and put_netmem() and does the
> synchronization between these 2 paths.
>
> When I have time I'll take a look to see if an LLM can find a better
> way to do this.

Hi Mina,

Thanks for the Reviewed-by.

I'll send v5 shortly addressing Jakub's requests. The deeper
restructure you're musing about seems orthogonal to this fix,
so let's discuss it separately.

Thanks,
Jijie Shao