Re: [PATCH net-next 10/10] net/mlx5e: Enable the data direct netdev feature

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net/mlx5e: unbind event ignores dmabuf bindings and channel switch failures
--

commit 9f56d4d1fade135dc92a72f9f099f77953de0c1d
Author: Dragos Tatulea <dtatulea@nvidia.com>

net/mlx5e: Enable the data direct netdev feature

This commit implements the data_direct private flag in the mlx5e driver,
enabling the feature. It also adds a check to prevent changing the flag's
state if there are active dmabuf bindings to avoid breaking them during
the resulting DMA device change.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> index b3eced1dd43fd..8ab1150ce3f20 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c

[ ... ]

> @@ -2402,7 +2403,40 @@ static int set_pflag_tx_port_ts(struct net_device *netdev, bool enable)
>  
>  static int set_pflag_data_direct(struct net_device *netdev, bool enable)
>  {
> -	return -EOPNOTSUPP;
> +	struct mlx5e_priv *priv = netdev_priv(netdev);
> +	struct mlx5_core_dev *mdev = priv->mdev;
> +	struct mlx5e_params new_params;
> +
> +	if (enable) {
> +		if (!mlx5_data_direct_supported(mdev) ||
> +		    !MLX5_CAP_GEN(mdev, crossing_vhca_mkey)) {
> +			netdev_err(netdev, "data_direct is not supported by the device\n");
> +			return -EOPNOTSUPP;
> +		}
> +
> +		if (!MLX5_CAP_GEN(mdev, shampo)) {
> +			netdev_err(netdev, "data_direct requires SHAMPO (Header-Data Split) support\n");
> +			return -EOPNOTSUPP;
> +		}
> +
> +		if (!mlx5_data_direct_get_dev(mdev->data_direct)) {
> +			netdev_err(netdev, "data_direct device not bound\n");
> +			return -ENODEV;
> +		}
> +	}
> +
> +	/* Changing the flag reopens the channels with a different DMA device,
> +	 * which would break any dmabuf currently bound to this netdev.
> +	 */
> +	if (netdev_has_dmabuf_binding(netdev, NULL)) {
> +		netdev_err(netdev, "Can't change data_direct while dmabufs are bound\n");
> +		return -EBUSY;
> +	}
> +
> +	new_params = priv->channels.params;
> +	MLX5E_SET_PFLAG(&new_params, MLX5E_PFLAG_DATA_DIRECT, enable);
> +
> +	return mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);
>  }

[Severity: High]
This ethtool callback explicitly acknowledges that reopening channels with a
different DMA device breaks dmabuf bindings. However, does the unbind notifier
in mlx5e_data_direct_event() omit this safety check?

Looking at mlx5e_data_direct_event() in
drivers/net/ethernet/mellanox/mlx5/core/en_main.c:

	new_params = priv->channels.params;
	MLX5E_SET_PFLAG(&new_params, MLX5E_PFLAG_DATA_DIRECT, false);

	mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);

	netdev_info(priv->netdev,
		    "data_direct device unbound, disabled data_direct\n");

The notifier forcefully reopens the channels without checking for or revoking
active dmabuf bindings. The channels are reopened using the netdev's primary
DMA device, but the dmabufs remain mapped for the removed data_direct GPU,
which could cause IOMMU faults.

Additionally, what happens if the channel switch in the notifier fails?

The return value of mlx5e_safe_switch_params() is ignored. If it fails
(e.g., due to ENOMEM), the old channels might continue running with the old
parameters, actively issuing DMA requests to the unbound and
soon-to-be-freed data_direct device, potentially leading to a
use-after-free and DMA corruption.

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