Re: [PATCH v4 net-next 13/14] net: enetc: use alloc_etherdev_mqs() to create netdev for VF driver

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing bounds checks on hardware-provided ring counts lead to heap buffer overflows in internal driver structures and undefined behavior via out-of-bounds pointer arithmetic.
--

--- Patch [13]: [PATCH v4 net-next 13/14] net: enetc: use alloc_etherdev_mqs() to create netdev for VF driver ---
commit 351253af314542011d0490ff56bc5d410a44e0d7
Author: Wei Fang <wei.fang@nxp.com>

net: enetc: use alloc_etherdev_mqs() to create netdev for VF driver

The VF driver uses alloc_etherdev_mq() with ENETC_MAX_NUM_TXQS as the
queue count, which forces the TX and RX queue counts to be equal and
uses a compile-time constant rather than the actual hardware capability.
This patch switches to alloc_etherdev_mqs() so that the TX and RX queue
counts are set independently, each capped at ENETC_MAX_NUM_TXQS, based
on the actual number of rings assigned to the VF by the PF.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_vf.c b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> index 9cdb0a4d6bafb..7dcb4a0246f52 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> @@ -317,7 +317,14 @@ static int enetc_vf_probe(struct pci_dev *pdev,
>  
>  	enetc_get_si_caps(si);
>  
> -	ndev = alloc_etherdev_mq(sizeof(*priv), ENETC_MAX_NUM_TXQS);
> +	/* Currently, the supported SoCs have a max of 6 CPUs and the VFs
> +	 * have less than 6 RX/TX rings. So no issues for these supported
> +	 * SoCs, but for future SoCs which have more CPUs or more TX/RX
> +	 * rings, all the related logic needs to be improved.
> +	 */
> +	ndev = alloc_etherdev_mqs(sizeof(*priv),
> +				  min(si->num_tx_rings, ENETC_MAX_NUM_TXQS),
> +				  min(si->num_rx_rings, ENETC_MAX_NUM_TXQS));

[Severity: High]
This is a pre-existing issue, but since we are relying on hardware-provided
values (si->num_tx_rings and si->num_rx_rings) read from ENETC_SICAPR0, is
it possible for a misconfigured or malicious PF/hypervisor to provide values
larger than the fixed-size arrays in struct enetc_ndev_priv?

In enetc_init_si_rings_params():

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

These values are then used in enetc_alloc_msix() which calls
enetc_int_vector_init(). Since priv->int_vector is sized to 6
(ENETC_MAX_BDR_INT) and priv->tx_ring is sized to 16, a VF providing a large
number of rings could cause out-of-bounds pointer writes:

drivers/net/ethernet/freescale/enetc/enetc.c:enetc_int_vector_init() {
	...
	priv->int_vector[i] = v;
	...
	priv->tx_ring[idx] = bdr;
	...
}

Additionally, in enetc_alloc_msix(), if priv->num_tx_rings is less than
num_possible_cpus(), does this calculation lead to out-of-bounds pointer
arithmetic?

drivers/net/ethernet/freescale/enetc/enetc.c:enetc_alloc_msix() {
	...
	first_xdp_tx_ring = priv->num_tx_rings - num_possible_cpus();
	priv->xdp_tx_ring = &priv->tx_ring[first_xdp_tx_ring];
	...
}

Would it be safer to explicitly validate si->num_tx_rings and
si->num_rx_rings against ENETC_MAX_BDR_INT and ARRAY_SIZE(priv->tx_ring)
early in the initialization rather than assuming they will always be small?

>  	if (!ndev) {
>  		err = -ENOMEM;
>  		dev_err(&pdev->dev, "netdev creation failed\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707081834.710730-1-wei.fang@oss.nxp.com?part=13