Re: [PATCH net 4/4] net/mlx5e: Fix publication race for priv->channel_stats[]

[Jacob Keller <jacob.e.keller@intel.com>]

On 6/4/2026 6:50 AM, Tariq Toukan wrote:
> From: Feng Liu <feliu@nvidia.com>
> 
> mlx5e_channel_stats_alloc() publishes a new entry to
> priv->channel_stats[] and then increments priv->stats_nch as a
> publication token, but neither store carries any memory barrier:
> 
> 	priv->channel_stats[ix] = kvzalloc_node(...);
> 	if (!priv->channel_stats[ix])
> 		return -ENOMEM;
> 	priv->stats_nch++;
> 
> Concurrent readers compute the loop bound from priv->stats_nch and
> then dereference priv->channel_stats[i] using plain accesses, e.g.
> 
> 	for (i = 0; i < priv->stats_nch; i++) {
> 		struct mlx5e_channel_stats *cs = priv->channel_stats[i];
> 		... cs->rq.packets ...
> 	}
> 
> On weakly-ordered architectures (ARM, PowerPC, RISC-V) the writes to
> channel_stats[ix] and stats_nch may become visible to other CPUs out
> of program order. A reader can observe stats_nch == N while still
> seeing channel_stats[N-1] == NULL, leading to a NULL pointer
> dereference in the channel_stats loop.
> 
> This has been observed in production on BlueField-3 DPUs (arm64),
> where ovs-vswitchd queries netdev statistics over netlink during NIC
> bringup, racing mlx5e_open_channel() -> mlx5e_channel_stats_alloc()
> on another CPU:
> 
>   Unable to handle kernel NULL pointer dereference at virtual address 0x840
>   Hardware name: BlueField-3 DPU
>   pc : mlx5e_fold_sw_stats64+0x30/0x180 [mlx5_core]
>   Call trace:
>    mlx5e_fold_sw_stats64+0x30/0x180 [mlx5_core]
>    dev_get_stats+0x50/0xc0
>    ovs_vport_get_stats+0x38/0xac [openvswitch]
>    ovs_vport_cmd_fill_info+0x194/0x290 [openvswitch]
>    ovs_vport_cmd_get+0xbc/0x10c [openvswitch]
>    genl_family_rcv_msg_doit+0xd0/0x160
>    genl_rcv_msg+0xec/0x1f0
>    netlink_rcv_skb+0x64/0x130
>    genl_rcv+0x40/0x60
>    netlink_unicast+0x2fc/0x370
>    netlink_sendmsg+0x1dc/0x454
>    ...
>    __arm64_sys_sendmsg+0x2c/0x40
> 
> Order the stats_nch increment through smp_store_release() in the
> writer, paired with smp_load_acquire() of stats_nch in every reader.
> The release/acquire pair establishes the contract:
> 
>   stats_nch == N  =>  channel_stats[0..N-1] are visible and non-NULL.
> 
> Update all readers of priv->stats_nch in mlx5e RX/TX queue stats,
> mlx5e_get_base_stats(), ethtool channels stats, IPoIB stats, the
> sw_stats fold and the HV VHCA stats agent to use smp_load_acquire().
> mlx5e_channel_stats_alloc() (the writer, serialized by state_lock)
> and mlx5e_priv_cleanup() (single-owner teardown) are intentionally
> not modified.
> 

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>