Re: [PATCH net v4 3/4] net: ethernet: oa_tc6: mdiobus->parent initialized with NULL

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free during driver removal due to missing devm teardown synchronization.
- [High] Hardware-induced kernel panic (DoS) due to unchecked `skb_put()` extending beyond allocated buffer bounds.
- [High] Race condition freezing the TX queue due to lockless `waiting_tx_skb` check.
--

commit 2c34943e07dcd7c39f87708d303b4633c83360bd
Author: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>

net: ethernet: oa_tc6: mdiobus->parent initialized with NULL
    
As "dev" pointer in oa_tc6 structure is never initialized,
mbiosub->parent was initialized with NULL.  This change
fixes the issue.

> diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
> index 20c9e98d1d607..0710a5a8dcdec 100644
> --- a/drivers/net/ethernet/oa_tc6.c
> +++ b/drivers/net/ethernet/oa_tc6.c
> @@ -108,7 +108,6 @@
>  
>  /* Internal structure for MAC-PHY drivers */
>  struct oa_tc6 {
> -	struct device *dev;
>  	struct net_device *netdev;
>  	struct phy_device *phydev;
>  	struct mii_bus *mdiobus;

[ ... ]

[Severity: High]
This is a pre-existing issue, but could this teardown sequence result in a
use-after-free crash?

In drivers/net/ethernet/microchip/lan865x/lan865x.c:lan865x_remove():

    unregister_netdev(priv->netdev);
    oa_tc6_exit(priv->tc6);
    free_netdev(priv->netdev);

The driver registers a threaded IRQ using devres. This means the IRQ is freed
after the remove() function completes.

Could the IRQ thread run concurrently with or after oa_tc6_exit() and
free_netdev(), accessing the freed SKBs and netdev structure since the active
background thread is not canceled before freeing its resources?

[Severity: High]
This isn't a bug introduced by this patch, but does this code properly
validate the buffer boundaries before appending data?

In drivers/net/ethernet/oa_tc6.c:oa_tc6_prcs_ongoing_rx_frame():

    oa_tc6_update_rx_skb(tc6, payload, OA_TC6_CHUNK_PAYLOAD_SIZE);

Which appends data unconditionally in oa_tc6_update_rx_skb():

    memcpy(skb_put(tc6->rx_skb, length), payload, length);

If a faulty or malicious MAC-PHY continuously sends data chunks without
asserting the end_valid flag, wouldn't skb_put() hit the end of the buffer
and trigger skb_over_panic() since there is no check against skb_tailroom()?

[Severity: High]
This is a pre-existing issue, but could this lockless check cause the TX
queue to freeze permanently?

In 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 in oa_tc6_try_spi_transfer() on the worker thread:

    if (!tc6->waiting_tx_skb && netif_queue_stopped(tc6->netdev))
        netif_wake_queue(tc6->netdev);

If the worker thread takes the lock, clears waiting_tx_skb, and evaluates
netif_queue_stopped() before start_xmit() actually stops the queue, the
worker thread skips waking the queue.

When start_xmit() resumes, it stops the queue and returns NETDEV_TX_BUSY.
Since waiting_tx_skb is now NULL and the worker missed the wake-up, is there
anything left to wake the queue?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-level-trigger-v4-0-6f389abdd192@onsemi.com?part=3