Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] netmem: devmem leak and kernel warning in nsim_pp_hold_write() --
commit b58e0da7daeddfad13177ed902471523d6b96cbf Author: Mina Almasry <[email protected]> netdevsim: support devmem RX and TX bindings Set a 64-bit DMA mask on nsim_bus_dev, enable NETMEM_TX_DMA on the netdevsim net_device, and configure page_pool instances with PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV | PP_FLAG_ALLOW_UNREADABLE_NETMEM and queue_idx so devmem bind-rx and bind-tx netlink operations can be tested against netdevsim 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] When PP_FLAG_ALLOW_UNREADABLE_NETMEM is set and devmem is bound, will this cause a leak and kernel warning in the debugfs handler? If nsim_pp_hold_write() is called to test allocating memory from the pool, it uses page_pool_dev_alloc_pages(): drivers/net/netdevsim/netdev.c: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 devmem may be bound, page_pool_alloc_netmems() can allocate an unreadable net_iov. When this is passed to netmem_to_page(), it detects an IOV, triggers a WARN_ON_ONCE(), and returns NULL. nsim_pp_hold_write() receives NULL and returns -ENOMEM, leaking the allocated devmem netmem_ref because it is neither stored nor freed. While this is restricted to root via debugfs, does this break the exact devmem testing functionality this patch intends to support? > + .max_len = PAGE_SIZE, > .nid = NUMA_NO_NODE, > - .dev = &napi->dev->dev, > + .dev = &ns->nsim_bus_dev->dev, > .napi = napi, > .dma_dir = DMA_BIDIRECTIONAL, > - .netdev = napi->dev, > + .netdev = dev, > + .queue_idx = idx, > }; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
