Re: [PATCH v2 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:
- [High] Dynamically allocating netdev queues based on unvalidated hardware ring counts bypasses previous implicit limits, leading to array overflows and unhandled interrupts (interrupt storms) for queues >= 8.
--

commit 25e82eb7e0ae74aa61ceb3d3b3628db74277e9eb
Author: Wei Fang <wei.fang@nxp.com>

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

The VF driver currently uses alloc_etherdev_mq() with a fixed queue count.
This commit changes it to use alloc_etherdev_mqs() so that TX and RX queue
counts can be set independently based on actual hardware capabilities.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_vf.c b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> index 9cdb0a4d6bafb..3df515a6e3335 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> @@ -317,7 +317,8 @@ static int enetc_vf_probe(struct pci_dev *pdev,
>  
>  	enetc_get_si_caps(si);
>  
> -	ndev = alloc_etherdev_mq(sizeof(*priv), ENETC_MAX_NUM_TXQS);
> +	ndev = alloc_etherdev_mqs(sizeof(*priv), si->num_tx_rings,
> +				  si->num_rx_rings);

[Severity: High]
By dynamically allocating the netdev queues based on unvalidated hardware
ring counts, can this bypass previous limits and cause array overflows?

Previously, alloc_etherdev_mq() capped the queue allocation to
ENETC_MAX_NUM_TXQS, which safely bounded the number of queues and would fail
if the hardware capabilities were excessive. If a PF or hypervisor
configures the VF with excessive queues (for instance, si->num_tx_rings > 8
or si->num_rx_rings > 6), wouldn't this allocation now succeed and lead to
out-of-bounds writes during initialization?

For example, in enetc_int_vector_init():

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

If si->num_rx_rings > 6 and num_online_cpus() > 6, doesn't this overflow
the priv->int_vector[] array? And if si->num_tx_rings > 16, won't this
overflow the fixed 16-element priv->tx_ring[] array?

Furthermore, for TX queues >= 8, it seems the MSI-X handler and NAPI poll
loop still use the hardcoded limit ENETC_MAX_NUM_TXQS (8):

drivers/net/ethernet/freescale/enetc/enetc.c:enetc_msix() {
    ...
    for_each_set_bit(i, &v->tx_rings_map, ENETC_MAX_NUM_TXQS)
    ...
}

And similarly in enetc_poll():

drivers/net/ethernet/freescale/enetc/enetc.c:enetc_poll() {
    ...
    for_each_set_bit(i, &v->tx_rings_map, ENETC_MAX_NUM_TXQS)
    ...
}

Will this fail to clear or re-enable interrupts for TX queues >= 8,
resulting in unhandled interrupts and an immediate system lockup?

Should we validate si->num_tx_rings and si->num_rx_rings against the
maximum sizes supported by the driver structures before allocating the
netdev?

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

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