Re: [PATCH can-next 09/13] can: gs_usb: gs_usb_state_get_by_hf(): optionally print recived CAN error frames

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The driver reads uninitialized kernel heap memory or stale URB data from `data[1]`, `data[6]`, and `data[7]` when processing short CAN error frames.
--

--- Patch [9]: [PATCH can-next 09/13] can: gs_usb: gs_usb_state_get_by_hf(): optionally print recived CAN error frames ---
commit f418a47ba868ccfb1b18e7b941a14de6bd026311
Author: Marc Kleine-Budde <[email protected]>

can: gs_usb: gs_usb_state_get_by_hf(): optionally print recived CAN error frames

This commit introduces a netdev_dbg() statement in gs_usb_state_get_by_hf()
to parse and print the current CAN error state and error frame payload,
making debugging easier across different gs_usb compatible implementations.

> diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
> index e41f91b56ab82..2848cb1a75f03 100644
> --- a/drivers/net/can/usb/gs_usb.c
> +++ b/drivers/net/can/usb/gs_usb.c
> @@ -670,6 +670,21 @@ gs_usb_state_get_by_hf(const struct gs_can *dev, const struct gs_host_frame *hf,
>  			else
>  				tx_state = CAN_STATE_ERROR_ACTIVE;
>  		}
> +
> +		const u8 tx_err = hf->classic_can->data[6];
> +		const u8 rx_err = hf->classic_can->data[7];

[Severity: Medium]
Could this lead to reading uninitialized kernel heap memory or stale URB data
if a device sends a short CAN error frame?

If a malfunctioning USB device sends an error frame where CAN_ERR_FLAG is set
but specifies a can_dlc of less than 8, gs_usb_get_minimum_rx_length() might
calculate a small minimum_length based on the untrusted can_dlc.

This allows short packets to bypass the bounds checks. When
gs_usb_state_get_by_hf() unconditionally accesses data[6] and data[7] here,
it can read beyond the actual received payload length.

> +
> +		netdev_dbg(dev->netdev, "host_frame: TX-state='%s/%u' RX-state='%s/%u' %s%s%s%s%s%s%s",
> +			   can_get_state_str(tx_state), tx_err,
> +			   can_get_state_str(rx_state), rx_err,
> +			   hf->can_id & cpu_to_le32(CAN_ERR_BUSOFF) ? "bus-off " : "",

[ ... ]

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