Re: [PATCH net 1/2] net/mlx5e: psp: Fix invalid access on PSP dev registration fail

[Cosmin Ratiu]

On Sat, 2026-04-18 at 12:08 -0700, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> net/mlx5e: psp: Fix invalid access on PSP dev registration fail
> 
> This commit updates PSP device registration to use a local variable,
> only saving the PSP device pointer if psp_dev_create() succeeds. This
> prevents an error pointer from being stored in priv->psp->psp.
> 
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c
> [ ... ]
> > @@ -1070,29 +1070,37 @@ static struct psp_dev_ops mlx5_psp_ops = {
> [ ... ]
> >  void mlx5e_psp_register(struct mlx5e_priv *priv)
> >  {
> > +	struct mlx5e_psp *psp = priv->psp;
> > +	struct psp_dev *psd;
> > +
> [ ... ]
> > -	priv->psp->psp = psp_dev_create(priv->netdev,
> > &mlx5_psp_ops,
> > -					&priv->psp->caps, NULL);
> > -	if (IS_ERR(priv->psp->psp))
> > +	psd = psp_dev_create(priv->netdev, &mlx5_psp_ops, &psp-
> > >caps, NULL);
> > +	if (IS_ERR(psd)) {
> >  		mlx5_core_err(priv->mdev, "PSP failed to register
> > due to %pe\n",
> > -			      priv->psp->psp);
> > +			      psd);
> > +		return;
> > +	}
> > +	psp->psp = psd;
> >  }
> 
> When psp_dev_create() fails, this function now returns without
> setting
> psp->psp, leaving it as NULL. However, priv->psp remains allocated
> and
> non-NULL.
> 
> Does this leave the RX datapath vulnerable to a NULL pointer
> dereference?
> 
> If priv->psp is non-NULL, the NIC RX initialization path can still
> call
> mlx5_accel_psp_fs_init_rx_tables(), which creates hardware flow
> steering
> rules to intercept UDP traffic.
> 
> If a UDP packet triggers these rules, the hardware flags the CQE with
> MLX5E_PSP_MARKER_BIT. The RX fast-path sees the marker and invokes
> mlx5e_psp_offload_handle_rx_skb(), which dereferences the pointer
> unconditionally:
> 
> u16 dev_id = priv->psp->psp->id;
> 
> Since priv->psp->psp is NULL, this will cause a kernel panic. Should
> priv->psp be cleaned up, or the error propagated, to prevent flow
> rules
> from being installed when registration fails?

First, this is preexisting. But more importantly, it's impossible to
trigger:
- with no PSP devs, there can be no PSP SAs installed.
- with no SAs, PSP decryption cannot succeed.
- all unsuccessfully decrypted PSP packets are dropped by steering.
- the RX handler will not see any PSP packets with the marker set.

This patch fixes the comparatively way more likely scenario of
psp_dev_register failing and then mlx5e_psp_unregister passing the
error pointer to psp_dev_unregister, which will do unpleasant things
with it.

Cosmin.