Re: [PATCH v5 09/10] can: peak_usb: register the netdevs only once all channels are set up

[Stéphane Grosjean]

On 22/10/2026 05:36, netdev-bot+sashiko@xxxxxxxxxx wrote:
[...]
> [Severity: Low]
> The comment says this also runs during probe, before the netdev is
> registered. The helpers this function calls still log through
> netdev_err(), though. Was the conversion meant to stop here?
[...]
> Should those helpers be converted as well?

These helpers are mostly used at run time, where the interface name is
useful, and during probe their messages still start with the driver and
USB device names. So I'd rather leave them as they are in this fix, and
clean them up later on, if needed.

[...]
> [Severity: Medium]
> This is a pre-existing issue, not introduced by this patch, but can
> args[0] be read here without ever being initialized?

Confirmed. v6 adds a new patch, "can: peak_usb: don't return an
uninitialized PCAN-USB CAN channel ID", which initializes args[]. The
PCAN-USB Pro and FD versions of this callback aren't affected.

[...]
> [Severity: Low]
> peak_usb_disconnect() now also cleans up netdevs that were never
> registered. The PCAN_USB_STATE_REGISTERED test only guards
> unregister_candev(). The name is still copied and printed for every
> sibling:
[...]
> Will each unregistered channel then log
> a literal "can%d removed"?

Right, on a failed probe only. This is cosmetic, so I'll fix it along
with the teardown issues below rather than in this series.

> [Severity: Medium]
> This isn't a bug introduced by this patch, but does the bus-off command
> sent on close ever reach the device on this path?
[...]
> [Severity: Low]
> This is a pre-existing issue, but the hardware state set by dev_init()
> does not seem to be undone on this path.

Both are right, and both predate this series: CONNECTED was already
cleared before ndo_stop, and dev_exit() is only called at module
unload. They only matter when the adapter stays plugged in (unbind, or
probe failure), and fixing them means sending commands to the device
once its RX is stopped, which needs some care. I'll address them in a
separate series.

> [Severity: High]
> This isn't a bug introduced by this patch, but can this teardown lead to
> a use-after-free? Neither this error path nor peak_usb_disconnect()
> drains the asynchronous restart URBs or cancels the PCAN-USB
> restart_timer before free_candev().

Confirmed: the restart URBs aren't anchored, and the PCAN-USB
restart_timer is never deleted. The window already exists on unplug
without this series, so I'll fix it in that same separate series, which
is about the teardown, rather than grow this one further.

> [Severity: High]
> This is a pre-existing issue in pcan_usb_pro.c and is not related to the
> code changed here. Can pcan_usb_pro_handle_canmsg() copy past the end of
> the record and past can_frame->data[]?

Yes, fixed by a new patch in v6, see my reply to patch 8.

Thanks!

Stéphane