Re: [PATCH net-next] net: devmem: use page_pool_put_netmem_bulk

On Thu, Sep 24, 2026 at 3:36 PM [off-list ref] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] sock_devmem_dontneed() (net/core/sock.c) now releases the last
  page_pool reference of devmem net_iovs via…

--

net: devmem: use page_pool_put_netmem_bulk

This converts sock_devmem_dontneed() from a loop of napi_pp_put_page()
calls to a single page_pool_put_netmem_bulk() call per batch, so the
page_pool ring producer lock is taken once per batch instead of once per
netmem.

quoted

diff --git a/net/core/sock.c b/net/core/sock.c
index 2948dffcc3e1b..a6b6d5b2051df 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c

[ ... ]

quoted

@@ -1123,8 +1124,7 @@ sock_devmem_dontneed(struct sock *sk, sockptr_t optval, unsigned int optlen)
                      netmems[netmem_num++] = netmem;
                      if (netmem_num == ARRAY_SIZE(netmems)) {
                              xa_unlock_bh(&sk->sk_user_frags);
-                             for (k = 0; k < netmem_num; k++)
-                                     WARN_ON_ONCE(!napi_pp_put_page(netmems[k]));
+                             page_pool_put_netmem_bulk(netmems, ARRAY_SIZE(netmems));

[Severity: High]
Can this reintroduce the page_pool use-after-free that commit
271683bb2cf32e ("page_pool: Fix use-after-free in
page_pool_recycle_in_ring") addressed, when CONFIG_PAGE_POOL_STATS=y?

The old path went through napi_pp_put_page() ->
page_pool_put_unrefed_netmem() -> page_pool_recycle_in_ring(), which
updates the stat while still holding the producer lock:

net/core/page_pool.c:page_pool_recycle_in_ring() {
        in_softirq = page_pool_producer_lock(pool);
        ret = !__ptr_ring_produce(&pool->ring, (__force void *)netmem);
        if (ret)
                recycle_stat_inc(pool, ring);
        page_pool_producer_unlock(pool, in_softirq);
}

The bulk path touches the pool after the unlock:

net/core/page_pool.c:page_pool_recycle_ring_bulk() {
        ...
        page_pool_producer_unlock(pool, in_softirq);
        recycle_stat_add(pool, ring, i);

Once the netmems are published into pool->ring, does the setsockopt
thread still hold anything that keeps the pool alive?  The barrier in
page_pool_release() only covers work done inside the producer lock:

net/core/page_pool.c:page_pool_release() {
        page_pool_scrub(pool);
        inflight = page_pool_inflight(pool, true);
        /* Acquire producer lock to make sure producers have exited. */
        in_softirq = page_pool_producer_lock(pool);
        page_pool_producer_unlock(pool, in_softirq);
        if (!inflight)
                __page_pool_destroy(pool);
}

so the sequence would be:

CPU0 (setsockopt SO_DEVMEM_DONTNEED)
  page_pool_put_netmem_bulk()
    page_pool_recycle_ring_bulk()
      __ptr_ring_produce()          /* last inflight refs published */
      page_pool_producer_unlock()   /* spin_unlock_bh(): BH/preempt window */

CPU1 (page_pool_release_retry() worker after deferred destroy)
  page_pool_release()
    page_pool_scrub() -> page_pool_empty_ring() -> page_pool_return_netmem()
    page_pool_inflight() == 0
    producer lock taken/released uncontended
    __page_pool_destroy() -> page_pool_uninit()
      free_percpu(pool->recycle_stats); kfree(pool);

CPU0
      recycle_stat_add(pool, ring, i);   /* this_cpu_add on freed memory */

Since the release is driven here by returning the last outstanding user
tokens, would it be better to move recycle_stat_add(pool, ring, i) inside
the producer-lock section of page_pool_recycle_ring_bulk() (matching
page_pool_recycle_in_ring()) before converting sock_devmem_dontneed() to
the bulk helper?

Thank you my dear bot. This seems like a real issue, a latent bug in
the *_bulk() put. Let me use my pet LLM to develop a fix for this,
send to net, and then respin this.

Lets hold onto this for now Kaifeng, and resubmit after my fix goes into net.

-- 
Thanks,
Mina