Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] networking-core: RCU Use-After-Free of queue stats arrays in mana_remove --
commit 18790cf70c36793724e3d46fb56949164d33f7af Author: Long Li <[email protected]> net: mana: keep per-queue statistics in the port context Prepare port-lifetime RX/TX statistics before introducing live queue replacement. Allocate max_queues slots before netdev registration, unwind probe failures, and release them only after unregistering the netdev. > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c > b/drivers/net/ethernet/microsoft/mana/mana_en.c > index 4b2c8039b8fa3..744d5ebe8dbbb 100644 > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c [ ... ] > @@ -597,15 +597,15 @@ static void mana_get_stats64(struct net_device *ndev, > struct rtnl_link_stats64 *st) > { > struct mana_port_context *apc = netdev_priv(ndev); > - unsigned int num_queues = apc->num_queues; > struct mana_stats_rx *rx_stats; > struct mana_stats_tx *tx_stats; > + unsigned int num_queues; > unsigned int start; > u64 packets, bytes; > int q; > > - if (!apc->port_is_up) > - return; > + /* Report even while down; dev_get_stats() zeroes its output. */ > + num_queues = apc->max_queues; > > netdev_stats_to_stats64(st, &ndev->stats); > > @@ -615,7 +615,18 @@ static void mana_get_stats64(struct net_device *ndev, > st->rx_missed_errors = apc->ac->hc_stats.hc_rx_discards_no_wqe; > > for (q = 0; q < num_queues; q++) { > - rx_stats = &apc->rxqs[q]->stats; > + rx_stats = &apc->rxq_stats[q]; > + > + do { > + start = u64_stats_fetch_begin(&rx_stats->syncp); [ ... ] > @@ -4525,6 +4591,7 @@ void mana_remove(struct gdma_dev *gd, bool suspending) > > unregister_netdevice(ndev); > mana_cleanup_indir_table(apc); > + mana_free_queue_stats(apc); [Severity: High] Is it possible for concurrent RCU readers to trigger a use-after-free of the queue stats arrays here? In mana_remove(), unregister_netdevice() unlists the device but does not synchronously wait for existing RCU grace periods to finish. The necessary synchronize_net() happens later in netdev_run_todo() after the lock is released. Because this patch removed the !apc->port_is_up safety check from mana_get_stats64(), concurrent RCU readers that have already acquired the ndev pointer can still call dev_get_stats() and unconditionally iterate and dereference the apc->rxq_stats and apc->txq_stats array pointers. If the arrays are freed by mana_free_queue_stats() while the reader is inside mana_get_stats64(), it would result in memory corruption or crashes. Does the mana_free_queue_stats() teardown need to be deferred until after the RCU grace period, such as in the ndo_uninit or destructor callback? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
