Re: [PATCH net v1 3/4] netdevsim: support devmem RX and TX bindings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] netdevsim: UAF in page_pool async destruction due to freed nsim_bus_dev
- [Medium] netdevsim: memory leak and page_pool release loop in nsim_pp_hold_write
--

commit 3929ee46c506ebcd379657adcaaf3d764e577c6d
Author: Mina Almasry <almasrymina@google.com>

netdevsim: support devmem RX and TX bindings

This patch sets a 64-bit DMA mask on nsim_bus_dev, enables NETMEM_TX_DMA,
and configures page_pool instances with PP_FLAG_DMA_MAP,
PP_FLAG_DMA_SYNC_DEV, and PP_FLAG_ALLOW_UNREADABLE_NETMEM to allow
testing of devmem bind operations in software selftests.

> diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
> index b4a99f3ceac60..4415dc01c610b 100644
> --- a/drivers/net/netdevsim/netdev.c
> +++ b/drivers/net/netdevsim/netdev.c

[ ... ]

> @@ -414,16 +414,22 @@ static int nsim_poll(struct napi_struct *napi, int budget)
>  	return done;
>  }
>  
> -static int nsim_create_page_pool(struct page_pool **p, struct napi_struct *napi)
> +static int nsim_create_page_pool(struct net_device *dev, struct page_pool **p,
> +				 struct napi_struct *napi, int idx)
>  {
> +	struct netdevsim *ns = netdev_priv(dev);
>  	struct page_pool_params params = {
>  		.order = 0,
>  		.pool_size = NSIM_RING_SIZE,
> +		.flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV |
> +			 PP_FLAG_ALLOW_UNREADABLE_NETMEM,

[Severity: Medium]
Does adding PP_FLAG_ALLOW_UNREADABLE_NETMEM here cause a memory leak when
used with the debugfs pp_hold file?

When devmem is bound, a user writing to pp_hold triggers nsim_pp_hold_write()
which attempts to allocate a page:

nsim_pp_hold_write() {
    ...
    } else if (val) {
        ns->page = page_pool_dev_alloc_pages(ns->rq[0]->page_pool);
        if (!ns->page)
            ret = -ENOMEM;
    ...
}

Because the page pool is configured to allow unreadable devmem,
page_pool_dev_alloc_pages() returns an unreadable devmem net_iov.
The inline function netmem_to_page() detects it is unreadable and returns
NULL.

nsim_pp_hold_write() sees the NULL, treats it as an allocation failure,
and drops the pointer without releasing the underlying net_iov back to the
page pool. This leaks the inflight netmem, and later causes the page pool
teardown workqueue (page_pool_release_retry) to spin indefinitely.

> +		.max_len = PAGE_SIZE,
>  		.nid = NUMA_NO_NODE,
> -		.dev = &napi->dev->dev,
> +		.dev = &ns->nsim_bus_dev->dev,

[Severity: High]
Can this lead to a use-after-free during asynchronous page pool destruction?

This sets the pool's device pointer to the dynamically allocated
nsim_bus_dev, but the page pool does not appear to take a reference to it.

If the netdevsim device is unregistered (e.g., via sysfs) while normal SKBs
are still inflight in a socket receive queue, nsim_bus_dev's refcount can
drop to 0, causing it to be synchronously freed:

nsim_bus_dev_release() {
    ...
    nsim_bus_dev = container_of(dev, struct nsim_bus_dev, dev);
    kfree(nsim_bus_dev);
}

When those inflight SKBs are eventually consumed and freed by user-space,
the asynchronous page pool destruction will invoke
__page_pool_unmap_netmem_dma(), which calls
dma_unmap_page_attrs(pool->p.dev, ...).

Because pool->p.dev points to the already-freed nsim_bus_dev->dev,
this results in a use-after-free.

> +		.napi = napi,
>  		.dma_dir = DMA_BIDIRECTIONAL,
> -		.netdev = napi->dev,
> +		.netdev = dev,
> +		.queue_idx = idx,
>  	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010025532.839559-1-almasrymina@google.com?part=3