Re: [PATCH can 3/3] can: gs_usb: gs_usb_receive_bulk_callback(): check actual_length before accessing data

[Henrik Brix Andersen]

Hi,

> On 8 Nov 2025, at 10.01, Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx> wrote:
> 
> The URB received in gs_usb_receive_bulk_callback() contains a struct
> gs_host_frame. The length of the data after the header depends on the
> gs_host_frame hf::flags and the active device features (e.g. time
> stamping).
> 
> Introduce a new function gs_usb_get_minimum_length() and check that we have
> at least received the required amount of data before accessing it. Only
> copy the data to that skb that has actually been received.
> 
> Fixes: d08e973a77d1 ("can: gs_usb: Added support for the GS_USB CAN devices")
> Signed-off-by: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> ---
> drivers/net/can/usb/gs_usb.c | 65 ++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 60 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
> index 51f8d694104d..8524d423b029 100644
> --- a/drivers/net/can/usb/gs_usb.c
> +++ b/drivers/net/can/usb/gs_usb.c
> @@ -261,6 +261,11 @@ struct canfd_quirk {
> u8 quirk;
> } __packed;
> 
> +/* struct gs_host_frame::echo_id == GS_HOST_FRAME_ECHO_ID_RX indicates
> + * a regular RX'ed CAN frame
> + */
> +#define GS_HOST_FRAME_ECHO_ID_RX 0xffffffff
> +
> struct gs_host_frame {
> struct_group(header,
> u32 echo_id;
> @@ -570,6 +575,43 @@ gs_usb_get_echo_skb(struct gs_can *dev, struct sk_buff *skb,
> return len;
> }
> 
> +static unsigned int
> +gs_usb_get_minimum_length(const struct gs_can *dev, const struct gs_host_frame *hf,
> +  unsigned int *data_length_p)
> +{
> + unsigned int minimum_length, data_length;
> +
> + /* TX echo only uses the header */
> + if (hf->echo_id != GS_HOST_FRAME_ECHO_ID_RX) {
> + *data_length_p = 0;
> + return sizeof(hf->header);
> + }

Is this correct? The embedded timestamp is also used in the gs_usb_get_echo_skb() function.

Regards,
Brix
-- 
Henrik Brix Andersen