Sashiko reports:
***
> err_config:
> + i40e_vsi_free_q_vectors(vsi);
> +err_qvec:
> i40e_vsi_clear_rings(vsi);
This is a pre-existing issue, but can the sequence in i40e_vsi_clear_rings()
lead to an RCU ordering violation?
In i40e_vsi_clear_rings(), the rings are freed before the array pointers are
nullified:
kfree_rcu(vsi->tx_rings[i], rcu);
WRITE_ONCE(vsi->tx_rings[i], NULL);
Under RCU rules, a pointer must be made unreachable to new readers before it
is handed off to kfree_rcu(). Could a new RCU reader (like
i40e_get_netdev_stats_struct_tx()) fetch the pointer after kfree_rcu() is
invoked, and access freed memory if the grace period expires while the
reader is still active?
***
Save the Tx ring pointer before clearing the published ring array slots
and pass the saved pointer to kfree_rcu(). This preserves the intended
RCU ordering, where new readers can no longer discover the ring through
vsi->tx_rings/rx_rings/xdp_rings before the object is queued for
deferred freeing, while avoiding a NULL kfree_rcu() argument after the
slot has already been cleared. Since the Tx pointer is the base of the
per-queue-pair allocation block, re-reading vsi->tx_rings[i] after
WRITE_ONCE(..., NULL) would otherwise turn the free into a no-op and
leak the whole ring block.
Fixes: 9f65e15b4f98 ("i40e: Move rings from pointer to array to array of
pointers")
Reported-by: Sashiko AI Review <[email protected]>
Signed-off-by: Maciej Fijalkowski <[email protected]>
---
drivers/net/ethernet/intel/i40e/i40e_main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c
b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 471fa7f7b643..a29a89192a7a 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -11699,11 +11699,13 @@ static void i40e_vsi_clear_rings(struct i40e_vsi *vsi)
if (vsi->tx_rings && vsi->tx_rings[0]) {
for (i = 0; i < vsi->alloc_queue_pairs; i++) {
- kfree_rcu(vsi->tx_rings[i], rcu);
+ struct i40e_ring *tx_ring = vsi->tx_rings[i];
+
WRITE_ONCE(vsi->tx_rings[i], NULL);
WRITE_ONCE(vsi->rx_rings[i], NULL);
if (vsi->xdp_rings)
WRITE_ONCE(vsi->xdp_rings[i], NULL);
+ kfree_rcu(tx_ring, rcu);
}
}
}
--
2.43.0
