Re: [PATCH v6 06/13] can: peak_usb: harden channel handling in PCAN-USB FD

[Stéphane Grosjean]

On 24/10/2026 05:58, netdev-bot+sashiko@xxxxxxxxxx wrote:
[...]
>>  adap_dev_free:
>> +	/* the whole interface is released when probing a channel fails:
>> +	 * stop RX on the channels already created, since their RX URBs may
>> +	 * carry records for this one.
>> +	 */
>> +	peak_usb_stop_rx(dev->prev_siblings);
>
> [Severity: High]
> Should this also stop RX on dev, and not only on its previous siblings?
> The comment above peak_usb_stop_rx() says it stops "dev and on all its
> previous siblings", but this call skips the channel that is failing.
>
> At this commit, peak_usb_create_dev() calls register_candev() before
> dev_init() and dev_set_bus(). So userspace may already have opened the
> failing channel, and its RX URBs may be in flight, when
> dev_set_bus(dev, 0) fails.
[...]
> Can the RX completions of this channel, and then pcan_usb_fd_stop(), use
> usb_if and the command buffer after they have been freed here?

Right, at this commit the failing channel itself may already be open.
v7 calls peak_usb_stop_rx(dev), which also stops RX on this channel, as
its comment says.

The rest of the sequence, dev_free() before unregister_candev() and
then pcan_usb_fd_stop() on the freed usb_if, is pre-existing: this
error path has always freed the adapter data before unregistering the
netdev. As you note, it is fixed by patch 9, after which no netdev can
be open when adap_dev_free runs.

[...]
> [Severity: Medium]
> This is a pre-existing issue, but when peak_usb_disconnect() is called
> from the probe error path, the device is still attached and its interface
> has not been disabled.
[...]
> After a failed probe, is the CAN controller left bus-on, while
> peak_usb_ndo_stop() treats the bus-off command as successful?

Yes, and as you note, the old loop already cleared CONNECTED just
before each unregister_candev(). After patch 9, it only happens if
register_candev() fails for a channel after a sibling has been
registered and opened, and also on an unbind through sysfs, with the
device still plugged.

Sending bus off (and the other commands of dev_exit()) on disconnect
when the device is still attached goes beyond this series, which keeps
the existing behavior here. I'll deal with it in a separate series, as
announced in the v6 cover letter.

[...]
> [Severity: Medium]
> This isn't a bug introduced by this patch, but the PCAN-USB Pro decoders
> look like they have the same problem that this patch fixes for PCAN-USB
> FD.
[...]
> The commit message does not mention PCAN-USB Pro. Two later patches in
> this series appear to fix this: "can: peak_usb: reject records for unset
> channels in PCAN-USB Pro" and "can: peak_usb: register the netdevs only
> once all channels are set up".

Yes, PCAN-USB Pro is handled by patches 8 and 9. This one is only about
PCAN-USB FD.

Thanks!

Stéphane

pw-bot: cr