Re: [PATCH v4 net 5/8] net: enetc: ensure head buffer has at least 16 octets

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] enetc: missing lock release on dropped one-step tstamp packets
--

commit 9392e5d35a9d4208a5ea777cd38ae2adf7ac8c86
Author: vladimir.oltean@nxp.com <vladimir.oltean@nxp.com>
net: enetc: ensure head buffer has at least 16 octets

This commit addresses a hardware issue in ENETC devices where transmitting
frames with a head buffer under 16 bytes causes an interrupt storm. It adds
a check in enetc_start_xmit to pull at least 16 bytes into the linear skb
header, dropping the packet if that fails.

Closes: https://lore.kernel.org/netdev/b3d9136c-2803-4203-b1ea-1f9e62de80a1@gmail.com/

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index 707c17fe4a4c5..89b47accbac84 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -1079,6 +1079,12 @@ static netdev_tx_t enetc_start_xmit(struct sk_buff *skb,
>  		if (eth_skb_pad(skb))
>  			return NETDEV_TX_OK;
>  
> +		if (unlikely(skb_headlen(skb) < ENETC_MIN_BUFF_SIZE &&
> +			     !pskb_may_pull(skb, ENETC_MIN_BUFF_SIZE))) {
> +			dev_kfree_skb_any(skb);
> +			return NETDEV_TX_OK;
> +		}
> +

[Severity: High]
Does this error path miss releasing the bit lock for one-step timestamp
packets?

Earlier in enetc_start_xmit(), the bit lock is acquired for packets with
the ENETC_F_TX_ONESTEP_SYNC_TSTAMP flag:

        if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
                if (test_and_set_bit_lock(ENETC_TX_ONESTEP_TSTAMP_IN_PROGRESS,
                                          &priv->flags)) {
                        skb_queue_tail(&priv->tx_skbs, skb);
                        return NETDEV_TX_OK;
                }
        }

If the newly added pskb_may_pull() check fails, the packet is freed and the
function returns NETDEV_TX_OK without clearing the previously acquired lock.

Could this leave the ENETC_TX_ONESTEP_TSTAMP_IN_PROGRESS bit permanently set,
causing subsequent one-step timestamp packets to queue infinitely to tx_skbs
without ever being dequeued?

>  		if (unlikely(skb_shinfo(skb)->nr_frags > priv->max_frags))
>  			if (unlikely(skb_linearize(skb)))
>  				goto drop_packet_err;

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