Re: [PATCH net-next v7 03/13] net: mana: keep per-queue statistics in the port context

[sashiko-bot@kernel.org]

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 <longli@microsoft.com>

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/cover.1791710735.git.weh@linux.microsoft.com?part=3