Re: [PATCH net-next v6 12/12] gve: add link status/speed ctrl ops

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] AdminQ commands that allocate a DMA buffer, send it to the device, and free it on timeout create a DMA-after-free vulnerability.
--

commit e2461d13b564589c9ca69a35d656bc9e72b3ea2b
Author: Joshua Washington <joshwash@google.com>

gve: add link status/speed ctrl ops

This commit refactors the link status check to use a control plane operation.
It introduces new operations for retrieving the link status and speed in AQ
mode, reading from the device status register.

> diff --git a/drivers/net/ethernet/google/gve/gve_ethtool.c b/drivers/net/ethernet/google/gve/gve_ethtool.c
> index dd1c44fedc771..a3220c6514382 100644
> --- a/drivers/net/ethernet/google/gve/gve_ethtool.c
> +++ b/drivers/net/ethernet/google/gve/gve_ethtool.c
> @@ -749,12 +749,11 @@ static int gve_get_link_ksettings(struct net_device *netdev,
>  	int err = 0;
>  
>  	if (priv->link_speed == 0)
> -		err = gve_adminq_report_link_speed(priv);
> +		err = priv->ctrl_ops->report_link_speed(priv);

[Severity: Critical]
This is a pre-existing issue, but does gve_adminq_report_link_speed() have a
DMA-after-free vulnerability when the AdminQ command times out?

Looking at the underlying implementation in gve_adminq_report_link_speed() in
drivers/net/ethernet/google/gve/gve_adminq.c:

    err = gve_adminq_execute_cmd(priv, &gvnic_cmd);

    priv->link_speed = be64_to_cpu(*link_speed_region);
    dma_free_coherent(&priv->pdev->dev, sizeof(*link_speed_region),
                      link_speed_region, link_speed_region_bus);
    return err;

If the hardware fails to respond within the loop in gve_adminq_wait_for_cmd(),
gve_adminq_execute_cmd() returns -ENOTRECOVERABLE. The driver then
unconditionally calls dma_free_coherent() on the link_speed_region buffer
while the hardware might still be processing the request.

If the hardware is merely slow and completes the operation after the driver's
timeout, it will perform a DMA write to the freed IOVA. This IOVA may have
been reallocated by the kernel for other purposes, leading to memory
corruption.

A similar pattern appears to exist in gve_adminq_execute_extended_cmd() where
the inner_cmd DMA buffer is unconditionally freed on failure:

    err = gve_adminq_execute_cmd(priv, &cmd);

    dma_free_coherent(&priv->pdev->dev, cmd_size, inner_cmd, inner_cmd_bus);
    return err;

Could this memory corruption be triggered by local users querying link
settings via ethtool if the vNIC is temporarily unresponsive?

>  
>  	cmd->base.speed = priv->link_speed;
>  
>  	cmd->base.duplex = DUPLEX_FULL;
> -
>  	return err;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909191136.3811892-1-hramamurthy@google.com?part=12