Re: [PATCH net 6/6] gve: fix NULL dereference from premature XSK pool DMA unmap

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

Pre-existing issues:
- [High] Kernel panic via NULL pointer dereference on XSK pool disable error path.
--

--- Patch [6]: [PATCH net 6/6] gve: fix NULL dereference from premature XSK pool DMA unmap ---
commit 70486a861c2e62f343665b0b44d80a866b2cbd87
Author: Joshua Washington <[email protected]>

gve: fix NULL dereference from premature XSK pool DMA unmap

This commit defers the XSK pool DMA unmapping in gve_xsk_pool_disable()
until after the NAPI rings are disabled, aiming to prevent a race
condition that causes a NULL pointer dereference.

> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> --- a/drivers/net/ethernet/google/gve/gve_main.c
> +++ b/drivers/net/ethernet/google/gve/gve_main.c
[ ... ]
> @@ -1681,28 +1681,24 @@ static int gve_xsk_pool_disable(struct net_device *dev,
>  	/* Stop and start RDA queues to repost buffers. */
>  	if (!gve_is_qpl(priv) && priv->xdp_prog) {
>  		err = gve_configure_rings_xdp(priv, priv->rx_cfg.num_queues);
>  		if (err)
> -			return err;
> +			goto unmap_and_return;

[Severity: High]
This is a pre-existing issue, but if gve_configure_rings_xdp() fails
(for example, due to a memory allocation failure), doesn't this goto bypass
the calls to napi_disable_locked() and gve_unreg_xsk_pool()?

[ ... ]
> -	return 0;
> +unmap_and_return:
> +	pool = xsk_get_pool_from_qid(dev, qid);
> +	if (pool)
> +		xsk_pool_dma_unmap(pool,
> +				   DMA_ATTR_SKIP_CPU_SYNC |
> +				   DMA_ATTR_WEAK_ORDERING);
> +
> +	return err;
>  }

[Severity: High]
If we reached this label via the gve_configure_rings_xdp() error path,
is it safe to call xsk_pool_dma_unmap() here?

Since the NAPI polling wasn't disabled and the active hardware ring's
xsk_pool pointer wasn't cleared, couldn't the still-active NAPI thread
dereference the unmapped pool->dev (which xsk_pool_dma_unmap() sets to
NULL) on the next packet, resulting in the exact same kernel panic this
patch is trying to prevent?

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