Re: [PATCH net-next v6 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 2 potential issue(s) to consider:
- [High] net: mana: UAF in ndo_get_stats64 due to premature free in mana_remove
- [Medium] net: mana: lost xdp_xmit stats due to lockless update in mana_xdp_xmit()
--

commit d18c42b1a688757d3c44ba3c118c5293e960c9e8
Author: Long Li <longli@microsoft.com>

net: mana: keep per-queue statistics in the port context

This patch prepares port-lifetime RX/TX statistics before introducing
live queue replacement. It allocates max_queues slots before netdev
registration and preserves counters while the interface is down.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_bpf.c b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
> index ff54f89668251..1905214bec487 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_bpf.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
> @@ -68,7 +68,7 @@ int mana_xdp_xmit(struct net_device *ndev, int n, struct xdp_frame **frames,
>  		count++;
>  	}
>  
> -	tx_stats = &apc->tx_qp[q_idx]->txq.stats;
> +	tx_stats = apc->tx_qp[q_idx]->txq.stats;
>  
>  	u64_stats_update_begin(&tx_stats->syncp);
>  	tx_stats->xdp_xmit += count;
>  	u64_stats_update_end(&tx_stats->syncp);

[Severity: Medium]
Can this result in lost statistic increments due to a data race?

When multiple CPUs perform XDP redirects to the same queue, they will execute
mana_xdp_xmit() concurrently. Because tx_stats->xdp_xmit is updated outside
of lock protection, the non-atomic addition operations can collide and
cause lost increments.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 45cb23717151f..02cd5f7656ed8 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c

[ ... ]

> @@ -4491,6 +4557,7 @@ void mana_remove(struct gdma_dev *gd, bool suspending)
>  
>  		unregister_netdevice(ndev);
>  		mana_cleanup_indir_table(apc);
> +		mana_free_queue_stats(apc);
>  
>  		/* Remove the port from reset walks before freeing its netdev.
>  		 */

[Severity: High]
Does this create a use-after-free race with lockless readers like
mana_get_stats64()?

When mana_remove() calls unregister_netdevice(), the synchronize_net()
call is deferred until rtnl_unlock() happens later. Because
mana_free_queue_stats() is called here before the RCU grace period completes,
concurrent readers querying the device could dereference the freed
apc->rxq_stats and apc->txq_stats pointers.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1790795005.git.weh@linux.microsoft.com?part=3