Re: [PATCH v4 net 2/8] net: enetc: serialize access to the same TX ring from NAPI XDP_TX and ndo_xdp_xmit()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] net/enetc: AB-BA deadlock between tx_ring->xdp_lock and enetc_mdio_lock
- [High] net/enetc: concurrent TX ring corruption from stack and XDP_REDIRECT
- [High] net: enetc: out-of-bounds array access in enetc_xdp_xmit()
--

commit 129585c95d671cf1c40cb76a4f76a3a478c00776
Author: vladimir.oltean@nxp.com <vladimir.oltean@nxp.com>

net: enetc: serialize access to the same TX ring from NAPI XDP_TX and ndo_xdp_xmit()

This commit introduces a spin lock to serialize access to the transmit ring
between the napi XDP_TX path and ndo_xdp_xmit(). It acquires the new
xdp_lock in both the transmit and receive paths.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index 26abc5fafd188..418be387af18b 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -1817,6 +1817,7 @@ int enetc_xdp_xmit(struct net_device *ndev, int num_frames,
>  	int err = 0;
>  
>  	tx_ring = priv->xdp_tx_ring[smp_processor_id()];
> +	spin_lock(&tx_ring->xdp_lock);

[Severity: High]
Could this lead to an out-of-bounds array access if the hardware provides
fewer transmit rings than num_possible_cpus()?

If priv->num_tx_rings < num_possible_cpus(), enetc_alloc_msix() evaluates
first_xdp_tx_ring to a negative index. This causes priv->xdp_tx_ring to
point to memory preceding the priv->tx_ring array.

While enetc_setup_xdp_prog() refuses to attach a local xdp program when
resources are insufficient, the destination device's ndo_xdp_xmit() can
still be invoked independently via cross-device XDP_REDIRECT from another
interface. Under those conditions, this unconditionally reads garbage
memory and crashes when acquiring tx_ring->xdp_lock.

[Severity: High]
Is there a risk of concurrent transmit ring corruption from the network stack?

If no local xdp program is attached to the interface,
enetc_num_stack_tx_queues() exposes all transmit rings (including the ones
pointed to by priv->xdp_tx_ring) to the core network stack. The stack
queues packets to these rings via enetc_start_xmit() using __netif_tx_lock,
rather than tx_ring->xdp_lock.

Concurrently, if an XDP_REDIRECT from another interface targets this one,
enetc_xdp_xmit() will modify the exact same transmit ring using
tx_ring->xdp_lock.

Will this mismatch in locking strategies lead to unsynchronized access to
the producer variables (next_to_use, tx_swbd[]), causing ring corruption?

[ ... ]

> @@ -2120,6 +2131,8 @@ static int enetc_clean_rx_ring_xdp(struct enetc_bdr *rx_ring,
>  	}
>  
>  out:
> +	spin_unlock(&tx_ring->xdp_lock);
> +
>  	rx_ring->next_to_clean = i;
>  
>  	rx_ring->stats.packets += rx_frm_cnt;
>  	rx_ring->stats.bytes += rx_byte_cnt;
>  
>  	if (xdp_redirect_frm_cnt) {
>  		enetc_unlock_mdio();
>  		xdp_do_flush();
>  		enetc_lock_mdio();
>  	}
>  
> -	if (xdp_tx_frm_cnt)
> +	if (xdp_tx_frm_cnt) {
> +		spin_lock(&tx_ring->xdp_lock);
>  		enetc_update_tx_ring_tail(tx_ring);
> +		spin_unlock(&tx_ring->xdp_lock);
> +	}

[Severity: High]
Does this create an AB-BA deadlock between tx_ring->xdp_lock and
enetc_mdio_lock?

In enetc_xdp_xmit(), the code establishes the lock order tx_ring->xdp_lock
followed by enetc_mdio_lock:

enetc_xdp_xmit()
   spin_lock(&tx_ring->xdp_lock);
   ...
   enetc_lock_mdio();

However, here in enetc_clean_rx_ring_xdp(), tx_ring->xdp_lock is released
at the out: label, but the enetc_mdio_lock remains held (or is re-acquired
after xdp_do_flush()). Then, if xdp_tx_frm_cnt > 0, it attempts to
re-acquire tx_ring->xdp_lock while enetc_mdio_lock is held.

Since enetc_mdio_lock is an rwlock_t, a pending writer (like an ethtool phy
command) will block new readers. A cpu holding xdp_lock will block waiting
for mdio_lock, while the cpu holding mdio_lock (read) spins indefinitely
for xdp_lock, creating a hard deadlock.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015105536.1692348-1-vladimir.oltean@nxp.com?part=2