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

[Cunhao Lu]

Hi,

> [Severity: Medium]
> Is this check enough to prevent the failure described in the commit
> message, "potentially re-enabling hardware while the driver is tearing it
> down"?

No, netif_running() alone does not serialize restart with ndo_stop(). A
restart that has already passed the check can still overlap with hardware
teardown. The commit message explicitly documents this limitation and the
need for drivers to cancel restart work before tearing down resources.

For Rockchip, the series already calls close_candev_cancel_restart_sync()
at the start of rkcanfd_stop(). This cancels pending work and waits for any
restart already in progress before disabling interrupts, freeing the IRQ
or stopping the controller. The final close_candev() remains after IRQ
teardown for final cancellation and echo cleanup.

As discussed earlier, this series limits the driver-side fix to Rockchip.
You are right that sja1000 and flexcan still have the window you describe.
Those pre-existing races need separate driver fixes; the core checks do
not resolve them. For Rockchip, the series uses the early-cancellation
approach you suggest rather than adding RTNL locking to the worker.

> [Severity: Low]
> This isn't a bug, but the comment above can_bus_off() still describes the
> old condition:
>
> /* CAN bus-off
>  *
>  * This functions should be called when the device goes bus-off to
>  * tell the netif layer that no more packets can be sent or received.
>  * If enabled, a timer is started to trigger bus-off recovery.
>  */
>
> Could it mention that recovery is now only scheduled when the interface
> is running?

Agreed. I will update the comment to refer to delayed work and state that
recovery is scheduled only when automatic recovery is enabled and the
interface is running. I will also clarify the running-interface condition
in the restart-ms documentation. If v5 is accepted, I will send a
separate documentation cleanup patch later.

Regards,
Cunhao Lu