Re: [PATCH v5] can: tcan4x5x: put tcan into sleep when removing driver

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

Pre-existing issues:
- [High] Polling timer Use-After-Free during device shutdown when configured without an IRQ.
--

--- Patch [1]: [PATCH v5] can: tcan4x5x: put tcan into sleep when removing driver ---
commit 2a7917c5ce38fcbd43e02a99be03a839492ce7ee
Author: Sean Nyekjaer <[email protected]>

can: tcan4x5x: put tcan into sleep when removing driver

This commit puts the tcan4x5x transceiver into sleep mode when the driver is
removed instead of leaving it in its operating mode. This reduces power
consumption once the driver is no longer bound to the device.

> diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
> index 31cc9d0abd453..bdb266be6dd37 100644
> --- a/drivers/net/can/m_can/tcan4x5x-core.c
> +++ b/drivers/net/can/m_can/tcan4x5x-core.c
[ ... ]
> @@ -543,7 +565,7 @@ static void tcan4x5x_can_remove(struct spi_device *spi)
>  
>  	m_can_class_unregister(&priv->cdev);
>  
> -	tcan4x5x_power_enable(priv->power, 0);
> +	tcan4x5x_power_enable(priv, 0);
>  
>  	m_can_class_free_dev(priv->cdev.net);

[Severity: High]
This isn't a bug introduced by this patch, but could this lead to a
use-after-free of the m_can_classdev structure when the device is operating
in polling mode (without an IRQ)?

When the interface is shut down prior to driver removal, m_can_close()
eventually calls m_can_disable_all_interrupts() which attempts to stop the
polling timer:

drivers/net/can/m_can/m_can.c:m_can_disable_all_interrupts() {
    ...
    if (!cdev->net->irq) {
        ...
        hrtimer_try_to_cancel(&cdev->hrtimer);
    }
    ...
}

Because hrtimer_try_to_cancel() does not block to wait for a concurrently
executing timer callback (m_can_polling_timer()) to complete, it returns -1
and allows the driver teardown sequence to proceed immediately.

If the driver unbinds and tcan4x5x_can_remove() calls m_can_class_free_dev()
while the timer callback is still executing on another CPU, won't the timer
callback access freed memory when it interacts with fields like
cdev->can.state?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1