Re: [PATCH net-next v3 1/2] net: mana: Use per-queue allocation for tx_qp to reduce allocation size

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Lockless functions (mana_get_stats64 and mana_xdp_xmit) can experience a TOCTOU Use-After-Free when accessing the dynamically allocated tx_qp arrays during driver teardown.
- [High] Unprotected updates to u64_stats sequence counters in mana_xdp_xmit can corrupt stats synchronization for readers, leading to a hard lockup.
--

commit a5fe13810d8b6aacf3c073e5e3abaddcaa39dd5d
Author: Aditya Garg <gargaditya@linux.microsoft.com>

net: mana: Use per-queue allocation for tx_qp to reduce allocation size

This commit converts tx_qp from a contiguous array allocation to an array of
pointers. It allocates each queue's mana_tx_qp individually via kvzalloc,
which provides vmalloc fallback and helps avoid allocation failures due to
memory fragmentation.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_bpf.c b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
> index 7697c9b52ed3..b5e9bb184a1d 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;

Can mana_xdp_xmit() be called concurrently on multiple CPUs that map to the
same q_idx?

If the number of online CPUs exceeds the number of TX queues,
smp_processor_id() % ndev->real_num_tx_queues will result in multiple CPUs
sharing the same queue index.

The subsequent call to u64_stats_update_begin() requires mutual exclusion
among writers. Concurrent writes without locks could corrupt the underlying
sequence counter, potentially leaving it permanently odd and causing readers
to spin infinitely.

Additionally, since this function runs locklessly, could this pointer
dereference race with the driver teardown path?

If a teardown thread clears port_is_up right after the early check in
mana_xdp_xmit(), and proceeds to call mana_destroy_txq(), it frees the tx_qp
elements via kvfree() without an intervening synchronize_net() or RCU grace
period. This could lead to a use-after-free when accessing txq.stats.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index a654b3699c4c..8adf72b96145 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c

[ ... ]

> @@ -614,7 +614,7 @@ static void mana_get_stats64(struct net_device *ndev,
>  	}
>  
>  	for (q = 0; q < num_queues; q++) {
> -		tx_stats = &apc->tx_qp[q].txq.stats;
> +		tx_stats = &apc->tx_qp[q]->txq.stats;

Could this suffer from the same use-after-free race with teardown as
mentioned above?

Since mana_get_stats64() runs locklessly, it can run concurrently with
teardown paths like mana_per_port_queue_reset_work_handler(). The tx_qp
array elements might be freed by mana_destroy_txq() while mana_get_stats64()
is attempting to read the stats.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260502074552.23857-1-gargaditya@linux.microsoft.com?part=1