[PATCH net v3 1/4] gve: Fix race between stats query and queue reconfig
flat view
WARM1d
From: Harshitha Ramamurthy <hramamurthy@google.com>
Date: 2026-10-23 20:47:10
Also in:
lkml, stable
Subsystem:
networking drivers, the rest · Maintainers:
Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds
From: Pin-yen Lin <redacted>
Fix a concurrency bug where network statistics queries race with queue
reconfiguration or interface teardown, causing a Use-After-Free (UAF) or
NULL pointer dereference.
Currently, statistics queries access priv->tx and priv->rx without
locking. When an interface is brought down or reconfigured,
gve_queues_mem_remove() frees these arrays. A concurrent stats query
can dereference freed queue structures and crash the kernel.
This issue was discovered and actually triggered by the
drivers/net/stats.py selftest. When an interface was brought down,
querying per-queue statistics via Netlink caused a kernel NULL pointer
dereference in gve_get_rx_queue_stats():
BUG: kernel NULL pointer dereference, address: 00000000000000a0
RIP: 0010:gve_get_rx_queue_stats+0x17/0x50 [gve]
Call Trace:
<TASK>
netdev_stat_queue_sum+0x11c/0x250
netdev_nl_stats_by_netdev+0x21b/0x2d0
netdev_nl_qstats_get_dumpit+0xd2/0x220
genl_dumpit+0x32/0x90
netlink_dump+0x1d2/0x450
__netlink_dump_start+0x1db/0x2a0
genl_family_rcv_msg_dumpit+0x9b/0x100
...
Additionally, concurrent stats queries during queue reconfiguration can
race with gve_queues_mem_remove(), causing a Use-After-Free.
Fix this by introducing stats_lock to serialize stats queries against
queue teardown. In gve_queues_mem_remove(), queue pointers are set to
NULL under stats_lock before freeing. Stats readers acquire the lock
and verify pointers are non-NULL before reading counters.
This change was tested using the DQO RDA queue format.
Cc: stable@vger.kernel.org
Fixes: 2e5e0932dff5 ("gve: add support for basic queue stats")
Co-developed-by: Debarghya Kundu <redacted>
Signed-off-by: Debarghya Kundu <redacted>
Signed-off-by: Pin-yen Lin <redacted>
Signed-off-by: Harshitha Ramamurthy <hramamurthy@google.com>
---
Changes in v3:
- Retitled from "gve: Add NULL pointer checks for per-queue statistics"
- Protect all stats queries and queue lifecycle updates with spinlock
(sashiko)
- Refined the commit message
drivers/net/ethernet/google/gve/gve.h | 2 ++
drivers/net/ethernet/google/gve/gve_main.c | 37 +++++++++++++++++++---
2 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/google/gve/gve.h b/drivers/net/ethernet/google/gve/gve.h
index b7ce6e8632db..104468c2c07f 100644
--- a/drivers/net/ethernet/google/gve/gve.h
+++ b/drivers/net/ethernet/google/gve/gve.h@@ -957,6 +957,8 @@ struct gve_priv { struct workqueue_struct *gve_wq; struct work_struct service_task; struct work_struct stats_report_task; + /* Synchronizes stats accesses and queue teardown */ + spinlock_t stats_lock; unsigned long service_task_flags; unsigned long state_flags;
diff --git a/drivers/net/ethernet/google/gve/gve_main.c b/drivers/net/ethernet/google/gve/gve_main.c
index 2437cd8ba633..26fdd22644f4 100644
--- a/drivers/net/ethernet/google/gve/gve_main.c
+++ b/drivers/net/ethernet/google/gve/gve_main.c@@ -69,6 +69,7 @@ static void gve_get_stats(struct net_device *dev, struct rtnl_link_stats64 *s) int num_tx_queues; int ring; + spin_lock_bh(&priv->stats_lock); num_tx_queues = gve_num_tx_queues(priv); if (priv->rx) { for (ring = 0; ring < priv->rx_cfg.num_queues; ring++) {
@@ -96,6 +97,7 @@ static void gve_get_stats(struct net_device *dev, struct rtnl_link_stats64 *s) s->tx_bytes += bytes; } } + spin_unlock_bh(&priv->stats_lock); } static int gve_alloc_flow_rule_caches(struct gve_priv *priv)
@@ -642,9 +644,13 @@ static void gve_queues_mem_remove(struct gve_priv *priv) struct gve_rx_alloc_rings_cfg rx_alloc_cfg = {0}; gve_get_curr_alloc_cfgs(priv, &tx_alloc_cfg, &rx_alloc_cfg); - gve_queues_mem_free(priv, &tx_alloc_cfg, &rx_alloc_cfg); + + spin_lock_bh(&priv->stats_lock); priv->tx = NULL; priv->rx = NULL; + spin_unlock_bh(&priv->stats_lock); + + gve_queues_mem_free(priv, &tx_alloc_cfg, &rx_alloc_cfg); } static void gve_free_control_plane_resources(struct gve_priv *priv)
@@ -1383,6 +1389,7 @@ static int gve_queues_start(struct gve_priv *priv, int err; /* Record new resources into priv */ + spin_lock_bh(&priv->stats_lock); priv->tx = tx_alloc_cfg->tx; priv->rx = rx_alloc_cfg->rx;
@@ -1396,10 +1403,11 @@ static int gve_queues_start(struct gve_priv *priv, priv->rx_pages_per_qpl = rx_alloc_cfg->pages_per_qpl; priv->header_split_enabled = rx_alloc_cfg->enable_header_split; priv->rx_cfg.packet_buffer_size = rx_alloc_cfg->packet_buffer_size; + gve_init_sync_stats(priv); + spin_unlock_bh(&priv->stats_lock); gve_tx_start_rings(priv, gve_num_tx_queues(priv)); gve_rx_start_rings(priv, rx_alloc_cfg->qcfg_rx->num_queues); - gve_init_sync_stats(priv); err = netif_set_real_num_tx_queues(dev, priv->tx_cfg.num_queues); if (err)
@@ -2266,6 +2274,7 @@ void gve_handle_report_stats(struct gve_priv *priv) if (!gve_get_report_stats(priv)) return; + spin_lock_bh(&priv->stats_lock); be64_add_cpu(&priv->stats_report->written_count, 1); /* tx stats */ if (priv->tx) {
@@ -2330,6 +2339,7 @@ void gve_handle_report_stats(struct gve_priv *priv) }; } } + spin_unlock_bh(&priv->stats_lock); } /* Handle NIC status register changes, reset requests and report stats */
@@ -2786,9 +2796,15 @@ 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; + spin_lock_bh(&priv->stats_lock); + if (!priv->rx) + goto unlock; + + rx = &priv->rx[idx]; + do { start = u64_stats_fetch_begin(&rx->statss); rx_stats->packets = rx->rpackets;
@@ -2796,20 +2812,32 @@ static void gve_get_rx_queue_stats(struct net_device *dev, int idx, rx_stats->alloc_fail = rx->rx_skb_alloc_fail + rx->rx_buf_alloc_fail; } while (u64_stats_fetch_retry(&rx->statss, start)); + +unlock: + spin_unlock_bh(&priv->stats_lock); } 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; + spin_lock_bh(&priv->stats_lock); + if (!priv->tx) + goto unlock; + + tx = &priv->tx[idx]; + do { start = u64_stats_fetch_begin(&tx->statss); tx_stats->packets = tx->pkt_done; tx_stats->bytes = tx->bytes_done; } while (u64_stats_fetch_retry(&tx->statss, start)); + +unlock: + spin_unlock_bh(&priv->stats_lock); } static void gve_get_base_stats(struct net_device *dev,
@@ -2935,6 +2963,7 @@ static int gve_probe(struct pci_dev *pdev, const struct pci_device_id *ent) } INIT_WORK(&priv->service_task, gve_service_task); INIT_WORK(&priv->stats_report_task, gve_stats_report_task); + spin_lock_init(&priv->stats_lock); priv->tx_cfg.max_queues = max_tx_queues; priv->rx_cfg.max_queues = max_rx_queues;
--
2.56.0.257.gf7497c7d27-goog