Re: [PATCH v3 net 2/4] net: hns3: fix page_pool leak in hns3_alloc_ring_memory error path

[netdev-bot+sashiko@kernel.org]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The commit message says the new cleanup in
  hns3_alloc_ring_memory()…

--

net: hns3: fix page_pool leak in hns3_alloc_ring_memory error path

When hns3_alloc_ring_buffers() fails for an RX ring, the out_with_desc
error path in hns3_alloc_ring_memory() frees the descriptor ring. It does
not destroy the page_pool that hns3_alloc_page_pool() just created. This
patch adds a guarded page_pool_destroy() to that error path.

> Add page_pool_destroy() to the out_with_desc error path, guarded by
> a NULL check, mirroring the cleanup done in hns3_free_ring_memory().

[Severity: Low]
This isn't a bug, but is hns3_free_ring_memory() the right name here? I
can't find a function with that name under
drivers/net/ethernet/hisilicon/hns3/.

The only other RX page_pool_destroy() is in hns3_fini_ring(). That function
calls hns3_free_desc() first and destroys the pool last:

drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:hns3_fini_ring() {
	hns3_free_desc(ring);
	devm_kfree(ring_to_dev(ring), ring->desc_cb);
    ...
	if (!HNAE3_IS_TX_RING(ring) && ring->page_pool) {
		page_pool_destroy(ring->page_pool);
		ring->page_pool = NULL;
	}
}

The new out_with_desc path does it in the opposite order. It destroys the
pool and sets ring->page_pool to NULL, then calls hns3_free_desc().

That order looks harmless today. By the time hns3_alloc_ring_buffers()
returns an error, it has already detached every buffer it attached. Each
desc_cb[i].dma is zero, so hns3_free_buffer_detach() returns early and
never hands a page back to the NULL pool.

Could the commit message name hns3_fini_ring() instead? It could also drop
the word "mirroring", since the order differs.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261030095633.377289-1-shaojijie%40huawei.com