Re: [PATCH net v2 1/4] gve: Add NULL pointer checks for per-queue statistics

[Pin-yen Lin]

On Fri, Apr 24, 2026 at 5:24 PM Harshitha Ramamurthy
<hramamurthy@xxxxxxxxxx> wrote:
>
> From: Debarghya Kundu <debarghyak@xxxxxxxxxx>
>
> gve_get_[tx/rx]_queue_stats references the [tx/rx] null rings when the
> link is down. Add NULL pointer checks to guard this
>
> This was discovered by drivers/net/stats.py selftest.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 2e5e0932dff5 ("gve: add support for basic queue stats")
> Signed-off-by: Debarghya Kundu <debarghyak@xxxxxxxxxx>
> Signed-off-by: Pin-yen Lin <treapking@xxxxxxxxxx>
> Signed-off-by: Harshitha Ramamurthy <hramamurthy@xxxxxxxxxx>
> ---
>  drivers/net/ethernet/google/gve/gve_main.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
> index 424d973c97f2..ef00d9ca1643 100644
> --- a/drivers/net/ethernet/google/gve/gve_main.c
> +++ b/drivers/net/ethernet/google/gve/gve_main.c
> @@ -2746,9 +2746,13 @@ static void gve_get_rx_queue_stats(struct net_device *dev, int idx,
>                                    struct netdev_queue_stats_rx *rx_stats)
>  {
>         struct gve_priv *priv = netdev_priv(dev);
> -       struct gve_rx_ring *rx = &priv->rx[idx];
> +       struct gve_rx_ring *rx;
>         unsigned int start;
>
> +       if (!priv->rx)
> +               return;

Sashiko says:

Does this NULL check safely prevent a use-after-free regression in lockless
paths?
Queue statistics can be accessed locklessly from the core network stack
(e.g., via ndo_get_stats64 under rcu_read_lock). If gve_close() is called
concurrently, gve_queues_mem_free() frees priv->rx and priv->tx using
kvfree() without an RCU grace period (e.g., synchronize_net()).
A concurrent reader could pass this NULL check, stall briefly, and then
dereference the freed array memory.

Sashiko also made a similar comment about the tx stats.

While this race between stats and gve_close() is a pre-existing issue,
we will address it in a separate patch in v3.

> +       rx = &priv->rx[idx];
> +
>         do {
>                 start = u64_stats_fetch_begin(&rx->statss);
>                 rx_stats->packets = rx->rpackets;
> @@ -2762,9 +2766,13 @@ static void gve_get_tx_queue_stats(struct net_device *dev, int idx,
>                                    struct netdev_queue_stats_tx *tx_stats)
>  {
>         struct gve_priv *priv = netdev_priv(dev);
> -       struct gve_tx_ring *tx = &priv->tx[idx];
> +       struct gve_tx_ring *tx;
>         unsigned int start;
>
> +       if (!priv->tx)
> +               return;
> +       tx = &priv->tx[idx];
> +
>         do {
>                 start = u64_stats_fetch_begin(&tx->statss);
>                 tx_stats->packets = tx->pkt_done;
> --
> 2.54.0.545.g6539524ca2-goog
>

Regards,
Pin-yen