Re: [PATCH net-next v4 07/16] net: ethernet: oa_tc6: Support for vendor specific MMS

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Dangling pointer in the timestamp queue causes a critical Use-After-Free during error recovery.
- [High] Use-After-Free when stopping the SPI kthread if it exited early due to an error.
- [High] Memory leak of pending timestamp network buffers during module teardown.
--

commit 68e8c3a781a6a1a8716a6a44f875d674c0ba927a
Author: Selvamani Rajagopal <Selvamani.Rajagopal@onsemi.com>

net: ethernet: oa_tc6: Support for vendor specific MMS

This commit allows vendors to map a custom memory map select (MMS) value
between 10 and 15 to MDIO_MMD_VEND1 as per the OPEN Alliance 10BASE-T1x
Serial Interface specification. It adds an API interface to perform this
mapping during initialization.

> diff --git a/drivers/net/ethernet/oa_tc6/oa_tc6.c b/drivers/net/ethernet/oa_tc6/oa_tc6.c
> index 9410cecfdc2aa..2f9ace655e81c 100644
> --- a/drivers/net/ethernet/oa_tc6/oa_tc6.c
> +++ b/drivers/net/ethernet/oa_tc6/oa_tc6.c
[ ... ]
> @@ -1281,6 +1294,7 @@ struct oa_tc6 *oa_tc6_init(struct spi_device *spi, struct net_device *netdev)
>  	SET_NETDEV_DEV(netdev, &spi->dev);
>  	mutex_init(&tc6->spi_ctrl_lock);
>  	spin_lock_init(&tc6->tx_skb_lock);
> +	tc6->vend1_mms = -EOPNOTSUPP;
>  	tc6->tx_ts_idx = OA_TC6_TTSCA_REG_ID;
>  	INIT_LIST_HEAD(&tc6->tx_ts_skb_q);

[Severity: High]
This is a pre-existing issue, but does unconditionally calling
kthread_stop() in oa_tc6_exit() risk a Use-After-Free?

If an error occurs during an SPI transfer, the kthread loop terminates and
exits early:

oa_tc6_spi_thread_handler() {
	while (likely(!kthread_should_stop())) {
		...
		if (ret) {
			...
			oa_tc6_cleanup_ongoing_tx_skb(tc6);
			...
			return ret;
		}
	}
	return 0;
}

When a kthread exits on its own, the kernel automatically frees its task
structure. However, oa_tc6_exit() unconditionally calls
kthread_stop(tc6->spi_thread) during module teardown. Could this dereference
the potentially freed task pointer?

[Severity: High]
This isn't a bug introduced by this patch, but does the module teardown
leak network buffers that were queued for hardware timestamping?

During teardown, oa_tc6_exit() frees the main transmission and reception
buffers but does not clean up the buffers in tx_ts_skb_q:

oa_tc6_exit() {
	...
	kthread_stop(tc6->spi_thread);
	dev_kfree_skb_any(tc6->ongoing_tx_skb);
	dev_kfree_skb_any(tc6->waiting_tx_skb);
	dev_kfree_skb_any(tc6->rx_skb);
}

If the hardware drops a packet and no interrupt is generated, or if the driver
is unloaded before the timestamp arrives, won't these stranded buffers leak and
prevent the networking core from releasing the device?

[Severity: Critical]
This is a pre-existing issue, but does oa_tc6_cleanup_ongoing_tx_skb() leave
a dangling pointer in the timestamp queue that causes a Use-After-Free?

When an SPI transfer fails, the buffer is freed on the error path to reclaim
memory:

oa_tc6_cleanup_ongoing_tx_skb() {
	if (tc6->ongoing_tx_skb) {
		tc6->netdev->stats.tx_dropped++;
		kfree_skb(tc6->ongoing_tx_skb);
		tc6->ongoing_tx_skb = NULL;
	}
}

However, the buffer is never unlinked from the tx_ts_skb_q. When the hardware
subsequently triggers a timestamp interrupt, the interrupt handler iterates
over the queue and dereferences the destroyed buffer:

oa_tc6_process_deferred_skb() {
	...
	list_for_each_entry_safe(skb, tmp, &tc6->tx_ts_skb_q, list) {
		... accesses freed skb ...
	}
	...
}

Could this list traversal logic process the dangling list pointer and crash
the kernel?

>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605-s2500-mac-phy-support-v4-0-de0fbc13c6d8@onsemi.com?part=7