Re: [PATCH net 13/14] can: usb: f81604: fix struct f81604_int_data size mismatch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect ordering of URB length and status checks causes fatal URB errors to be bypassed, leading to infinite resubmission loops or spurious warnings on device disconnect.
--

commit 7de2dcdbebd610316c61044116af8d2dda19bd1f
Author: Ji-Ze Hong (Peter Hong) <peter_hong@fintek.com.tw>

can: usb: f81604: fix struct f81604_int_data size mismatch

This commit removes the __aligned(4) attribute from struct f81604_int_data,
changing its size from 12 bytes back to 9 bytes. This ensures that the valid
9-byte interrupt URBs sent by the hardware are no longer incorrectly dropped
by the short URB check.

Link: https://patch.msgid.link/20260824-f81604-fix-v2-1-fc9be5581394@fintek.com.tw
Closes: https://lore.kernel.org/all/A3834A07-5639-4779-844F-C5843DFC3928@dynetrex.com/

> diff --git a/drivers/net/can/usb/f81604.c b/drivers/net/can/usb/f81604.c
> index f12318268e46b..4c147b9d6d690 100644
> --- a/drivers/net/can/usb/f81604.c
> +++ b/drivers/net/can/usb/f81604.c
> @@ -169,7 +169,7 @@ struct f81604_int_data {
>  	u8 rxerr;
>  	u8 txerr;
>  	u8 val;
> -} __packed __aligned(4);
> +} __packed;

[Severity: High]
This is a pre-existing issue, but I wanted to ask about the ordering of
the URB length and status checks in f81604_read_int_callback().

When a hardware error occurs or the device is disconnected, the USB core
completes the interrupt URB with a fatal status (like -EPROTO or -ESHUTDOWN)
and sets urb->actual_length to 0.

Because f81604_read_int_callback() checks the length before the status:

	if (urb->actual_length < sizeof(*data)) {
		netdev_warn(netdev, "%s: short int URB: %u < %zu\n",
			    __func__, urb->actual_length, sizeof(*data));
		goto resubmit_urb;
	}

	switch (urb->status) {
	...
	case -ESHUTDOWN:
		return;
	...

Will this cause fatal URB errors to bypass the intended abort logic and loop
infinitely into resubmit_urb, or produce spurious warnings upon device
disconnect?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826121036.2706424-1-mkl@pengutronix.de?part=13