Re: [PATCH v2 2/3] can: ixxat_usb: add support for USB-to-CAN CL1 (V2) devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] can: ixxat_usb: FORTIFY_SOURCE panic due to field-spanning memcpy
--

commit 8f14df1317ca74accd08abf72e9dcfad2f429804
Author: Stéphane Grosjean <s.grosjean@peak-system.fr>

can: ixxat_usb: add support for USB-to-CAN CL1 (V2) devices

This commit introduces support for the IXXAT USB-to-CAN CL1 communication
layer, used by the legacy USB-to-CAN compact/embedded/professional family.
It wires up the corresponding device entries and control caps.

> diff --git a/drivers/net/can/usb/ixxat_usb/ixxat_usb_cl1.c b/drivers/net/can/usb/ixxat_usb/ixxat_usb_cl1.c
> new file mode 100644
> index 0000000000000..61d1f5dc44075
> --- /dev/null
> +++ b/drivers/net/can/usb/ixxat_usb/ixxat_usb_cl1.c

[ ... ]

> +static int ixxat_usb_get_ctrl_caps(struct ixxat_usb_candevice *dev,
> +				   struct ixxat_cancaps2 *caps)
> +{
> +	struct ixxat_usb_getcaps_cl1_cmd cmd = { 0 };
> +	const u16 port = dev->ctrl_index;
> +	const u32 req_size = sizeof(cmd.req);
> +	const u32 rcv_size = sizeof(cmd) - req_size;
> +	const u32 cmd_size = req_size + sizeof(cmd.res);
> +	int err;
> +
> +	ixxat_usb_setup_cmd(&cmd.req, &cmd.res);
> +	cmd.req.code = cpu_to_le32(IXXAT_USB_CAN_CMD_GETCAPS);
> +	cmd.req.port = cpu_to_le16(port);
> +	cmd.res.res_size = cpu_to_le32(rcv_size);
> +	memset(&cmd.caps, 0, sizeof(cmd.caps));
> +
> +	err = ixxat_usb_send_cmd(dev, port,
> +				 &cmd.req, cmd_size,
> +				 &cmd.res, rcv_size,
> +				 IXXAT_USB_CMD_TIMEOUT);

[Severity: High]
Does this cause a fortify bounds check panic when the payload is copied?

Since we are passing strongly-typed pointers to the .req and .res fields,
but passing sizes (cmd_size = 24, rcv_size = 48) that exceed those fields
(which are only 12 bytes), the memcpy() inside ixxat_usb_send_cmd_internal()
will fail the bounds check on __member_size(p).

When the kernel is compiled with fortify bounds checking, copying 48 bytes
into a 12-byte struct field will trigger a runtime panic. Is there a way to
pass a pointer to the outer cmd object, or use a flexible array to avoid the
strict bounds check?

[ ... ]

> +static int ixxat_usb_init_ctrl(struct ixxat_usb_candevice *dev)
> +{
> +	struct ixxat_usb_init_cl1_cmd cmd = { 0 };
> +	const u16 port = dev->ctrl_index;
> +	const u32 rcv_size = sizeof(cmd.res);
> +	const u32 cmd_size = sizeof(cmd);
> +	const struct can_bittiming *bt = &dev->can.bittiming;
> +	u8 opmode = IXXAT_USB_OPMODE_EXTENDED | IXXAT_USB_OPMODE_STANDARD;
> +	u8 btr0 = ((bt->brp - 1) & 0x3f) | (((bt->sjw - 1) & 0x3) << 6);
> +	u8 btr1 = ((bt->prop_seg + bt->phase_seg1 - 1) & 0xf) |
> +		  (((bt->phase_seg2 - 1) & 0x7) << 4);

[ ... ]

> +	ixxat_usb_setup_cmd(&cmd.req, &cmd.res);
> +	cmd.req.size = cpu_to_le32(cmd_size - rcv_size);
> +	cmd.req.code = cpu_to_le32(IXXAT_USB_CAN_CMD_INIT);
> +	cmd.req.port = cpu_to_le16(port);
> +	cmd.mode = opmode;
> +	cmd.btr0 = btr0;
> +	cmd.btr1 = btr1;
> +
> +	return ixxat_usb_send_cmd(dev, port,
> +				  &cmd.req, cmd_size,
> +				  &cmd.res, rcv_size,
> +				  IXXAT_USB_CMD_TIMEOUT);

[Severity: High]
Similarly here, passing a pointer to the 12-byte .req field but using a 
cmd_size of 28 bytes violates the object boundary of cmd.req. 

Does this also trigger a fortify bounds check panic when the device is probed
and the CAN controller is initialized?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916-ixxat_usb_can-v2-0-9b892e362d9e@peak-system.fr?part=2