Re: [PATCH net-next 02/13] net/mlx5: E-switch, do not leave an unpaired devcom registered

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] net/mlx5: UAF on unpaired devcom in sqs2vport_start
- [High] net/mlx5: NULL pointer dereference in mlx5e_tc_query_route_vport
--

commit f795a21baba8fe76b75fbf1109d13ce32a85a0d0
Author: Shay Drory <shayd@nvidia.com>
net/mlx5: E-switch, do not leave an unpaired devcom registered

This commit ensures that an unpaired eswitch offloads devcom component is
not left registered if the initial PAIR event fails. It calls
mlx5_esw_offloads_devcom_cleanup() to release the devcom component when
pairing is unsuccessful, avoiding incorrect state retention.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> index eb74b62601682..e7d92d9bde16d 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
[ ... ]
> @@ -3572,10 +3573,12 @@ void mlx5_esw_offloads_devcom_init(struct mlx5_eswitch *esw,
>  	if (!esw->devcom)
>  		return;
>  
> -	mlx5_devcom_send_event(esw->devcom,
> -			       ESW_OFFLOADS_DEVCOM_PAIR,
> -			       ESW_OFFLOADS_DEVCOM_UNPAIR,
> -			       esw);
> +	err = mlx5_devcom_send_event(esw->devcom,
> +				     ESW_OFFLOADS_DEVCOM_PAIR,
> +				     ESW_OFFLOADS_DEVCOM_UNPAIR,
> +				     esw);
> +	if (err)
> +		mlx5_esw_offloads_devcom_cleanup(esw);

[Severity: High]
Does this newly added cleanup path dynamically free the devcom component
while concurrent operations on registered netdevs might be accessing it?

In esw_offloads_enable(), mlx5_eswitch_enable_pf_vf_vports() registers
representor netdevs, making them visible to userspace. If the pairing
fails here, mlx5_esw_offloads_devcom_cleanup() immediately frees
esw->devcom (without an RCU grace period).

Because the representor netdevs are already registered, a concurrent
ndo_open (often triggered automatically by udev) will execute
mlx5e_sqs2vport_start(). This function accesses the devcom pointer
locklessly:

drivers/net/ethernet/mellanox/mlx5/core/en_rep.c:mlx5e_sqs2vport_start() {
    ...
    if (mlx5_devcom_comp_is_ready(esw->devcom) &&
        mlx5_devcom_for_each_peer_begin(esw->devcom))
        devcom_locked = true;
    ...
}

Could this race condition lead to accessing freed memory, triggering a UAF?

[Severity: High]
Can this also cause a NULL pointer dereference later on?

If the initial PAIR event fails, mlx5_esw_offloads_devcom_cleanup() is
called, setting esw->devcom = NULL. However, mlx5_esw_offloads_devcom_init()
returns void, so its caller esw_offloads_enable() ignores the failure and
successfully enables offloads mode.

When offloads are enabled, a user with CAP_NET_ADMIN can add a TC rule with
an encapsulation forwarding destination, causing the driver to invoke
mlx5e_tc_query_route_vport(), which blindly iterates over esw->devcom:

drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:mlx5e_tc_query_route_vport() {
    ...
    mlx5_devcom_for_each_peer_entry_rcu(esw->devcom, esw, pos) {
    ...
}

Because esw->devcom is NULL, mlx5_devcom_get_next_peer_data_rcu()
unconditionally dereferences devcom->comp:

drivers/net/ethernet/mellanox/mlx5/core/lib/devcom.c:mlx5_devcom_get_next_peer_data_rcu() {
    ...
    struct mlx5_devcom_comp *comp = devcom->comp;
    ...
}

Would this result in a deterministic crash?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923103830.1183-1-tariqt@nvidia.com?part=2