Re: [PATCH net v5 1/6] can: dev: prevent bus-off recovery when the interface is not running

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] can: dev: TOCTOU race in can_restart() allows accessing hardware with disabled clocks
--

commit 4e524e53df8c23d434660cf0e7c4868293520f98
Author: Cunhao Lu <1579567540@qq.com>

can: dev: prevent bus-off recovery when the interface is not running

This commit adds a check for netif_running() in can_restart() and
can_bus_off() to avoid executing bus-off recovery on an interface that
is in the process of closing.

> diff --git a/drivers/net/can/dev/dev.c b/drivers/net/can/dev/dev.c
> index 769745e22a3c7..92492c503446e 100644
> --- a/drivers/net/can/dev/dev.c
> +++ b/drivers/net/can/dev/dev.c
> @@ -198,6 +198,9 @@ static int can_restart(struct net_device *dev)
>  	if (!priv->do_set_mode)
>  		return -EOPNOTSUPP;
>  
> +	if (!netif_running(dev))
> +		return -ENETDOWN;
> +

[Severity: High]
Can this lockless check create a time-of-check to time-of-use (TOCTOU)
race that results in accessing hardware with disabled clocks?

If a bus-off event schedules the delayed restart work, can_restart()
could check netif_running() and evaluate to true. If the workqueue thread
is preempted immediately after this check, and concurrently the interface
is brought down:

dev_close() clears __LINK_STATE_START and calls ndo_stop().
In drivers like at91_can and rcar_can, ndo_stop() disables hardware
clocks before calling close_candev() to cancel the restart work:

drivers/net/can/at91_can.c:at91_close() {
    ...
    clk_disable_unprepare(priv->clk);
    phy_power_off(priv->transceiver);

    close_candev(dev);
    ...
}

Since close_candev() blocks on cancel_delayed_work_sync(), it waits for
can_restart() to finish. When the preempted workqueue thread resumes,
it has already passed the check and will proceed to call
priv->do_set_mode().

Will this attempt to access hardware registers while their backing clocks
are disabled, potentially causing a synchronous external abort or system
panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-rockchip-canfd-restart-close-v5-0-ae77327037c9@qq.com?part=1