Re: [PATCH v5 08/10] can: peak_usb: reject records for unset channels in PCAN-USB Pro

[Stéphane Grosjean]

On 22/10/2026 05:36, netdev-bot+sashiko@xxxxxxxxxx wrote:
[...]
> [Severity: Low]
> Should a well-formed record for an unset channel cause the rest of the
> transfer to be dropped?
[...]
> Would it be better to skip just this record, for example by returning 0?
> The same applies to the new check in pcan_usb_pro_handle_error().
>
> Later in the series, "can: peak_usb: register the netdevs only once all
> channels are set up" moves register_candev() into peak_usb_probe(),
> after all channels are initialized. That seems to make this branch
> unreachable by the end of the series.

Right, the rest of the transfer is dropped, as for the other malformed
records, and as patch 6 does for the PCAN-USB FD. I kept it that way on
purpose: as you note, patch 9 makes this branch unreachable, so this
only matters to a tree that would take this patch without patch 9. The
v6 changelog says so.

[...]
> [Severity: High]
> This is a pre-existing issue and was not introduced by this patch. Since
> this function is being changed anyway: can the payload memcpy() further
> down in pcan_usb_pro_handle_canmsg() read past the record and past the
> RX buffer?
[...]
> Could the code
> reject len > CAN_MAX_DLEN? It could also reject any length larger than
> the record format provides: 0 for RXMSG0 and RXRTR, 4 for RXMSG4, and 8
> for RXMSG8.

Confirmed. v6 adds a new patch, "can: peak_usb: validate the data
length of PCAN-USB Pro CAN records", which converts the DLC with
can_cc_dlc2len() and rejects a data frame whose length exceeds the
number of data bytes of its record type. Tested on a PCAN-USB Pro:
frames of all lengths are still received.

> [Severity: Medium]
> This isn't a bug introduced by this patch, but does the caller,
> pcan_usb_pro_decode_buf(), read pr->data_type before checking rec_ptr
> against msg_end?

Confirmed too, and pcan_usb_pro_wait_rsp() had the same issue, with a
32-bit record count and no bound at all. Another new patch in v6, "can:
peak_usb: don't read past the message in PCAN-USB Pro decoders", stops
both loops at the end of the received data.

[...]
> [Severity: High]
> This is a pre-existing issue and was not introduced by this patch, but
> can usb_if still be used after this kfree() for channel 0?
[...]
> Later in the series, "can: peak_usb: register the netdevs only once all
> channels are set up" removes register_candev() and the
> lbl_unregister_candev label from peak_usb_create_dev(). After that, no
> netdev can be open when adap_dev_free runs, which seems to fix this.

Agreed, this is fixed by patch 9 of the same series.

Thanks!

Stéphane