Re: [PATCH net-next v3 2/5] net: enetc: Enable probing of version 4.4 devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] net: enetc: out-of-bounds write to int_vector[] in enetc_int_vector_init()
--

commit 6de2e15f2fb313f3ab9a84d0bfd5a867de11c72c
Author: Claudiu Manoil <claudiu.manoil@nxp.com>

net: enetc: Enable probing of version 4.4 devices

This commit adds driver data and platform info entries for ENETC v4.4 PF and
VF devices on S32N79 SoCs with a 500 MHz system clock frequency.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index c6571b5cc8425..b989823859ff2 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -3832,6 +3832,20 @@ static const struct enetc_drvdata enetc4_vf_data = {
>  	.eth_ops = &enetc_vf_ethtool_ops,
>  };
>  
> +static const struct enetc_drvdata enetc4_s32n79_ppm_data = {
> +	.sysclk_freq = ENETC_CLK_500M,
> +	.tx_csum = true,
> +	.max_frags = ENETC4_MAX_SKB_FRAGS,
> +	.eth_ops = &enetc4_ppm_ethtool_ops,
> +};
> +
> +static const struct enetc_drvdata enetc4_s32n79_vf_data = {
> +	.sysclk_freq = ENETC_CLK_500M,
> +	.tx_csum = true,
> +	.max_frags = ENETC4_MAX_SKB_FRAGS,
> +	.eth_ops = &enetc_vf_ethtool_ops,
> +};
> +
>  static const struct enetc_platform_info enetc_info[] = {
>  	{ .revision = ENETC_REV_1_0,
>  	  .dev_id = ENETC_DEV_ID_PF,
> @@ -3862,6 +3876,18 @@ static const struct enetc_platform_info enetc_info[] = {
>  	  .dev_id = NXP_ENETC_VF_DEV_ID,
>  	  .data = &enetc4_vf_data,
>  	},
> +	{ .revision = ENETC_REV_4_4,
> +	  .dev_id = NXP_ENETC_PPM_DEV_ID,
> +	  .data = &enetc4_s32n79_ppm_data,
> +	},
> +	{ .revision = ENETC_REV_4_4,
> +	  .dev_id = NXP_ENETC_PF_DEV_ID,
> +	  .data = &enetc4_s32n79_ppm_data,
> +	},
> +	{ .revision = ENETC_REV_4_4,
> +	  .dev_id = NXP_ENETC_VF_DEV_ID,
> +	  .data = &enetc4_s32n79_vf_data,
> +	},

[Severity: Critical]
Can probing ENETC v4.4 on an 8-core S32N79 SoC overflow priv->int_vector[] in
enetc_int_vector_init()?

In drivers/net/ethernet/freescale/enetc/enetc.h, ENETC_MAX_BDR_INT is still
defined as 6:

#define ENETC_MAX_BDR_INT	6 /* fixed to max # of available cpus */
...
struct enetc_ndev_priv {
	...
	int bdr_int_num; /* number of Rx/Tx ring interrupts */
	struct enetc_int_vector *int_vector[ENETC_MAX_BDR_INT];
	u16 num_rx_rings, num_tx_rings;
	u16 rx_bd_count, tx_bd_count;

Previously supported SoCs had at most 6 CPUs, whereas S32N79 has 8 CPUs and
enetc4_default_rings_allocation() allocates up to ENETC_SI_MAX_RING_NUM (8)
RX rings to the PF.

When enetc_init_si_rings_params() runs on an 8-CPU system with 7 or 8 RX
rings:

	int cpus = num_online_cpus();
	...
	priv->num_rx_rings = min_t(int, cpus, si->num_rx_rings);
	priv->num_tx_rings = si->num_tx_rings;
	priv->bdr_int_num = priv->num_rx_rings;

priv->bdr_int_num becomes 7 or 8 without being clamped to ENETC_MAX_BDR_INT.
Then enetc_alloc_msix() -> enetc_int_vector_init() writes past the end of
priv->int_vector[]:

	for (i = 0; i < priv->bdr_int_num; i++) {
		...
		err = enetc_int_vector_init(priv, i, num_tx_rings);

where enetc_int_vector_init() does:

	priv->int_vector[i] = v;

overwriting num_rx_rings, num_tx_rings, rx_bd_count, tx_bd_count, and
subsequent fields in struct enetc_ndev_priv.

Should ENETC_MAX_BDR_INT be increased (and/or priv->bdr_int_num bounded by
ENETC_MAX_BDR_INT)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793712309.git.claudiu.manoil@nxp.com?part=2