Re: [PATCH net-next v3 03/13] net: ethernet: oa_tc6: add OA_TC6_BROKEN_PHY quirk flag

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] TOCTOU race in TX path can lead to a permanently stopped network queue.
--

commit 3d52460cfb1b1b89ef4ee5edf6fd1d68050769d6
Author: Ciprian Regus <ciprian.regus@analog.com>

net: ethernet: oa_tc6: add OA_TC6_BROKEN_PHY quirk flag

This commit adds the OA_TC6_BROKEN_PHY quirk flag to skip internal PHY
initialization and management for MAC-PHY devices with hardware issues.
Drivers can use this flag to manage the PHY themselves instead of relying
on the framework.

> diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
> index baba5aad84dfa..2a72f0c4b0095 100644
> --- a/drivers/net/ethernet/oa_tc6.c
> +++ b/drivers/net/ethernet/oa_tc6.c
> @@ -134,6 +134,7 @@ struct oa_tc6 {
>  	bool rx_buf_overflow;
>  	bool int_flag;
>  	bool prot_ctrl;
> +	enum oa_tc6_quirk_flag quirk_flags;
>  };
>  
>  enum oa_tc6_header_type {

[Severity: High]
This is a pre-existing issue and was not introduced by the patch under
review, but is there a TOCTOU race in the TX path that could lead to a
permanently stopped network queue?

If CPU1 runs oa_tc6_start_xmit() and sees tc6->waiting_tx_skb:

drivers/net/ethernet/oa_tc6.c:oa_tc6_start_xmit() {
	if (tc6->waiting_tx_skb) {
		netif_stop_queue(tc6->netdev);
		return NETDEV_TX_BUSY;
	}
}

And CPU2 concurrently runs the SPI kthread which clears the SKB and tries
to wake the queue:

drivers/net/ethernet/oa_tc6.c:oa_tc6_try_spi_transfer() {
		if (!tc6->waiting_tx_skb && netif_queue_stopped(tc6->netdev))
			netif_wake_queue(tc6->netdev);
}

Could the following sequence occur?

1. CPU1 checks tc6->waiting_tx_skb and evaluates to true.
2. CPU2 processes the skb and sets tc6->waiting_tx_skb to NULL.
3. CPU2 checks netif_queue_stopped(), which is false because CPU1 hasn't
   stopped it yet, so it skips waking the queue.
4. CPU1 calls netif_stop_queue() and returns NETDEV_TX_BUSY.

The SPI thread would then go to sleep and the network queue would remain
permanently stopped until an asynchronous event awakens the SPI thread.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604-adin1140-driver-v3-0-5debdb3173c4@analog.com?part=3