Re: [PATCH net-next V3 07/15] net/mlx5e: macsec: Move RX marker from ft_metadata to flow_tag

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Kernel panic in error path of MACsec rx/tx rule addition due to unconditional cleanup of uninitialized IDs.
--

commit 427d364a8074fd7354a5aec874b383f2ef24a88c
Author: Cosmin Ratiu <cratiu@nvidia.com>

net/mlx5e: macsec: Move RX marker from ft_metadata to flow_tag

This commit moves the MACsec RX marker from ft_metadata to flow_tag to
avoid conflicts with PSP SPI values. It defines a new marker for MACsec in
flow_tag, updating steering rules while keeping fs_id in ft_metadata.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
> index efc5167493c12..4ca3a6ded2841 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/macsec_fs.c
[ ... ]
> @@ -1803,6 +1805,11 @@ macsec_fs_rx_add_rule(struct mlx5_macsec_fs *macsec_fs,
>  
>  		macsec_fs_rx_setup_fte(spec, &flow_act, attrs, false);
>  
> +		spec->flow_context.flags |= FLOW_CONTEXT_HAS_TAG;
> +		spec->flow_context.flow_tag =
> +			FIELD_PREP(MLX5E_ACCEL_FLOW_TAG_PROTO_MASK,
> +				   MLX5E_ACCEL_FLOW_TAG_PROTO_MACSEC);
> +
>  		flow_act.modify_hdr = modify_hdr;
>  		flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST |
>  				  MLX5_FLOW_CONTEXT_ACTION_CRYPTO_DECRYPT |

[Severity: High]
This is a pre-existing issue, but does the error path in macsec_fs_rx_add_rule()
have a potential NULL pointer dereference?

If macsec_fs_rx_add_rule() fails before macsec_fs_id_add() is called (for
example, if mlx5_add_flow_rules() fails), the error cleanup path unconditionally
calls macsec_fs_id_del() via macsec_fs_rx_del_rule():

err:
	macsec_fs_rx_del_rule(macsec_fs, rx_rule, macsec_ctx->secy->netdev, fs_id);
	macsec_rule = NULL;

If this happens on the very first rule added for a device, the device hasn't
been added to macsec_devices_list yet. In macsec_fs_id_del(), this causes
macsec_device to remain NULL:

	WARN_ON(!macsec_device);
	
	fs_id_xa = (is_tx) ? &macsec_device->tx_id_xa :
			     &macsec_device->rx_id_xa;
	xa_lock(fs_id_xa);
	fs_id_found = xa_load(fs_id_xa, fs_id);

This dereferences the NULL macsec_device when generating the address for
fs_id_xa, which is then passed to xa_lock(), leading to a kernel panic.

Could this be triggered under memory pressure or if firmware rejects a rule
creation request?

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