Re: [PATCH net-next v3 01/15] gve: don't pass in unused parameter to gve_adminq_free

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

Pre-existing issues:
- [High] Memory leak of the adminq DMA pool during device reset due to an early return in gve_adminq_free().
--

--- Patch [1]: [PATCH net-next v3 01/15] gve: don't pass in unused parameter to gve_adminq_free ---
commit 3b6d21bb1589b652e4e7777b1b6be55fff25b9f0
Author: Harshitha Ramamurthy <[email protected]>

gve: don't pass in unused parameter to gve_adminq_free

This commit cleans up gve_adminq_free by removing an unused parameter.

> diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
> index 08587bf40ed4a..70ffed8b52c3d 100644
> --- a/drivers/net/ethernet/google/gve/gve_adminq.c
> +++ b/drivers/net/ethernet/google/gve/gve_adminq.c
[ ... ]
> @@ -385,7 +385,7 @@ void gve_adminq_release(struct gve_priv *priv)
>  	gve_clear_admin_queue_ok(priv);
>  }
>  
> -void gve_adminq_free(struct device *dev, struct gve_priv *priv)
> +void gve_adminq_free(struct gve_priv *priv)
>  {
>  	if (!gve_get_admin_queue_ok(priv))
>  		return;

[Severity: High]
This isn't a bug introduced by this patch, but does returning early here
cause a memory leak of the adminq DMA pool during a device reset?

When a reset happens, gve_reset_and_teardown() calls gve_trigger_reset()
which clears the admin queue OK flag. It then calls
gve_teardown_priv_resources() which calls gve_adminq_free():

drivers/net/ethernet/google/gve/gve_main.c:gve_reset_and_teardown() {
	...
	gve_trigger_reset(priv);
	/* With the reset having already happened, close cannot fail */
	if (was_up)
		gve_close(priv->dev);
	gve_teardown_priv_resources(priv);
	...
}

Because the OK flag is cleared, gve_adminq_free() returns early, skipping
the dma_pool_free() and dma_pool_destroy() calls, leaking the allocations.
During subsequent recovery, gve_init_priv() calls gve_adminq_alloc() which
overwrites priv->adminq_pool and priv->adminq with new allocations.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1