[PATCH v2] can: usb: f81604: fix struct f81604_int_data size mismatch

[PATCH v2] can: usb: f81604: fix struct f81604_int_data size mismatch

[Ji-Ze Hong via B4 Relay]

From: "Ji-Ze Hong (Peter Hong)" <peter_hong@fintek.com.tw>

The struct f81604_int_data defines 9 bytes of interrupt data:
- Byte 0: Status register (sr)
- Byte 1: Interrupt register (isrc)
- Byte 2: Interrupt enable register (ier)
- Byte 3: Arbitration lost capture (alc)
- Byte 4: Error code capture (ecc)
- Byte 5: Error warning limit register (ewlr)
- Byte 6: RX error counter (rxerr)
- Byte 7: TX error counter (txerr)
- Byte 8: Reserved (val)

The hardware sends exactly 9 bytes for the interrupt endpoint.
However, the struct was defined with __aligned(4) attribute which
caused the compiler to pad the struct to 12 bytes.

This causes a problem in f81604_read_int_callback() where the short
URB check compares urb->actual_length against sizeof(*data). When
sizeof(struct f81604_int_data) is 12 but the hardware only sends 9
bytes, the check fails and valid interrupt messages are discarded.

This results in the driver only being able to transmit once because
the TX complete interrupt is never processed.

Fix this by removing the __aligned(4) attribute so the struct size
matches the actual hardware data size of 9 bytes.

Fixes: 7299b1b39a25 ("can: usb: f81604: handle short interrupt urb messages properly")
Cc: stable@vger.kernel.org
Reported-by: Dynetrex, Admin <admin@dynetrex.com>
Closes: https://lore.kernel.org/all/A3834A07-5639-4779-844F-C5843DFC3928@dynetrex.com/
Signed-off-by: Ji-Ze Hong (Peter Hong) <peter_hong@fintek.com.tw>
---
v2:
- Added Reported-by and Remove mismatched Fixes tags
---
 drivers/net/can/usb/f81604.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/usb/f81604.c b/drivers/net/can/usb/f81604.c
index f12318268e46..4c147b9d6d69 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;
 
 struct f81604_sff {
 	__be16 id;

---
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260824-f81604-fix-aaebd1f42e06

Best regards,
--  
Ji-Ze Hong (Peter Hong) <peter_hong@fintek.com.tw>

Re: [PATCH v2] can: usb: f81604: fix struct f81604_int_data size mismatch

[Greg Kroah-Hartman]

On Mon, Aug 24, 2026 at 09:18:14PM +0800, Ji-Ze Hong via B4 Relay wrote:
> From: "Ji-Ze Hong (Peter Hong)" <peter_hong@fintek.com.tw>
> 
> The struct f81604_int_data defines 9 bytes of interrupt data:
> - Byte 0: Status register (sr)
> - Byte 1: Interrupt register (isrc)
> - Byte 2: Interrupt enable register (ier)
> - Byte 3: Arbitration lost capture (alc)
> - Byte 4: Error code capture (ecc)
> - Byte 5: Error warning limit register (ewlr)
> - Byte 6: RX error counter (rxerr)
> - Byte 7: TX error counter (txerr)
> - Byte 8: Reserved (val)
> 
> The hardware sends exactly 9 bytes for the interrupt endpoint.
> However, the struct was defined with __aligned(4) attribute which
> caused the compiler to pad the struct to 12 bytes.
> 
> This causes a problem in f81604_read_int_callback() where the short
> URB check compares urb->actual_length against sizeof(*data). When
> sizeof(struct f81604_int_data) is 12 but the hardware only sends 9
> bytes, the check fails and valid interrupt messages are discarded.
> 
> This results in the driver only being able to transmit once because
> the TX complete interrupt is never processed.
> 
> Fix this by removing the __aligned(4) attribute so the struct size
> matches the actual hardware data size of 9 bytes.
> 
> Fixes: 7299b1b39a25 ("can: usb: f81604: handle short interrupt urb messages properly")
> Cc: stable@vger.kernel.org
> Reported-by: Dynetrex, Admin <admin@dynetrex.com>
> Closes: https://lore.kernel.org/all/A3834A07-5639-4779-844F-C5843DFC3928@dynetrex.com/
> Signed-off-by: Ji-Ze Hong (Peter Hong) <peter_hong@fintek.com.tw>
> ---
> v2:
> - Added Reported-by and Remove mismatched Fixes tags

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH v2] can: usb: f81604: fix struct f81604_int_data size mismatch

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 1909 bytes --]

On 24.08.2026 21:18:14, Ji-Ze Hong via B4 Relay wrote:
> From: "Ji-Ze Hong (Peter Hong)" <peter_hong@fintek.com.tw>
>
> The struct f81604_int_data defines 9 bytes of interrupt data:
> - Byte 0: Status register (sr)
> - Byte 1: Interrupt register (isrc)
> - Byte 2: Interrupt enable register (ier)
> - Byte 3: Arbitration lost capture (alc)
> - Byte 4: Error code capture (ecc)
> - Byte 5: Error warning limit register (ewlr)
> - Byte 6: RX error counter (rxerr)
> - Byte 7: TX error counter (txerr)
> - Byte 8: Reserved (val)
>
> The hardware sends exactly 9 bytes for the interrupt endpoint.
> However, the struct was defined with __aligned(4) attribute which
> caused the compiler to pad the struct to 12 bytes.
>
> This causes a problem in f81604_read_int_callback() where the short
> URB check compares urb->actual_length against sizeof(*data). When
> sizeof(struct f81604_int_data) is 12 but the hardware only sends 9
> bytes, the check fails and valid interrupt messages are discarded.
>
> This results in the driver only being able to transmit once because
> the TX complete interrupt is never processed.
>
> Fix this by removing the __aligned(4) attribute so the struct size
> matches the actual hardware data size of 9 bytes.
>
> Fixes: 7299b1b39a25 ("can: usb: f81604: handle short interrupt urb messages properly")
> Cc: stable@vger.kernel.org
> Reported-by: Dynetrex, Admin <admin@dynetrex.com>
> Closes: https://lore.kernel.org/all/A3834A07-5639-4779-844F-C5843DFC3928@dynetrex.com/
> Signed-off-by: Ji-Ze Hong (Peter Hong) <peter_hong@fintek.com.tw>

Applied to linux-can.

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2] can: usb: f81604: fix struct f81604_int_data size mismatch

[Drew Willey]

Tested running ChromeOS, kernel 6.12, rev facaadd, f81604 (2c42:1709), the driver hits "f81604_read_int_callback: short int URB: 9 < 12" on the first Tx attempt and stops the Tx queue. Subsequent frames queue up and are not transmitted.

Reloaded the f81604 module with this patch. Transmitted several frames over a minute and all succeed.

Tested-by: Drew Willey <dwilley@google.com>

Re: [PATCH v2] can: usb: f81604: fix struct f81604_int_data size mismatch

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 6465 bytes --]

On 24.08.2026 21:18:14, Ji-Ze Hong via B4 Relay wrote:
> From: "Ji-Ze Hong (Peter Hong)" <peter_hong@fintek.com.tw>
>
> The struct f81604_int_data defines 9 bytes of interrupt data:
> - Byte 0: Status register (sr)
> - Byte 1: Interrupt register (isrc)
> - Byte 2: Interrupt enable register (ier)
> - Byte 3: Arbitration lost capture (alc)
> - Byte 4: Error code capture (ecc)
> - Byte 5: Error warning limit register (ewlr)
> - Byte 6: RX error counter (rxerr)
> - Byte 7: TX error counter (txerr)
> - Byte 8: Reserved (val)
>
> The hardware sends exactly 9 bytes for the interrupt endpoint.
> However, the struct was defined with __aligned(4) attribute which
> caused the compiler to pad the struct to 12 bytes.
>
> This causes a problem in f81604_read_int_callback() where the short
> URB check compares urb->actual_length against sizeof(*data). When
> sizeof(struct f81604_int_data) is 12 but the hardware only sends 9
> bytes, the check fails and valid interrupt messages are discarded.
>
> This results in the driver only being able to transmit once because
> the TX complete interrupt is never processed.
>
> Fix this by removing the __aligned(4) attribute so the struct size
> matches the actual hardware data size of 9 bytes.
>
> Fixes: 7299b1b39a25 ("can: usb: f81604: handle short interrupt urb messages properly")
> Cc: stable@vger.kernel.org
> Reported-by: Dynetrex, Admin <admin@dynetrex.com>
> Closes: https://lore.kernel.org/all/A3834A07-5639-4779-844F-C5843DFC3928@dynetrex.com/
> Signed-off-by: Ji-Ze Hong (Peter Hong) <peter_hong@fintek.com.tw>

I hope this patch goes into v7.3. While reviewing this patch the bot
found this issue:

Will the 2nd issue be fixed by
https://lore.kernel.org/all/20260901-f81604-fix-v1-1-c55b5178f9f8@fintek.com.tw/ ?

regards,
Marc

--- Patch [3]: [PATCH net 3/3] usb: f81604: fix struct f81604_int_data size mismatch ---
commit 6877926768a247e156dd43002e5a7320e9ac2d39
Author: Ji-Ze Hong (Peter Hong) <peter_hong@fintek.com.tw>

usb: f81604: fix struct f81604_int_data size mismatch

This commit drops the __aligned(4) attribute from struct f81604_int_data.
With the attribute gone, sizeof() is 9 bytes, which matches what the
interrupt endpoint sends. With the old 12-byte padded size, the short URB
check in f81604_read_int_callback() threw away every valid interrupt
message, so TX completion was never processed.

Link: https://patch.msgid.link/20260824-f81604-fix-v2-1-fc9be5581394@fintek.com.tw

> 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;

Discovered by: opus-5-5 · Missed by: gpt-6-astra, sashiko-gemini · Confirmed by: gpt-6-astra · Ranked High by: gpt-6-astra
This isn't a bug introduced by this patch, but there is a problem in the
short URB check that this patch adjusts. In f81604_read_int_callback(),
the check runs before the status switch:

drivers/net/can/usb/f81604.c:f81604_read_int_callback() {
    ...
	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 0: /* success */
		break;

	case -ENOENT:
	case -EPIPE:
	case -EPROTO:
	case -ESHUTDOWN:
		return;
    ...
}

Suppose the URB completes with one of these error codes and actual_length
is 0. Won't it take the goto resubmit_urb path and never reach the
return?

On every ndo_stop, this call chain completes the interrupt URB with
-ENOENT and 0 bytes:

f81604_close()->f81604_unregister_urbs()->usb_kill_anchored_urbs()

The callback would then log a false "short int URB" warning. The
resubmit would fail with -EPERM because usb_kill_urb() has set
urb->reject, and that logs "failed to resubmit int urb".

On -EPIPE (endpoint stall) or -EPROTO, the URB would be resubmitted at
interval 1 instead of stopping. Each failing completion then logs
netdev_info and netdev_warn messages with no rate limit.

This ordering came from 7299b1b39a25, the commit this patch fixes. This
patch doesn't change the behaviour, since 0 < 9 just as 0 < 12.

Could the length check be applied only when urb->status is 0?

Discovered by: gpt-6-astra · Missed by: opus-5-5, sashiko-gemini · Confirmed by: opus-5-5 · Ranked Critical by: gpt-6-astra
This is a pre-existing issue, but this patch makes it reachable again.
f81604_close() cancels clear_reg_work before it kills the interrupt URBs
that can queue that work:

drivers/net/can/usb/f81604.c:f81604_close() {
    ...
	netif_stop_queue(netdev);
	cancel_work_sync(&priv->clear_reg_work);
	close_candev(netdev);

	f81604_unregister_urbs(priv);
    ...
}

What if an interrupt URB completes after cancel_work_sync() returns but
before usb_kill_anchored_urbs() runs? Can f81604_handle_can_bus_errors()
then queue the work again after a bus error, arbitration loss or overrun?

drivers/net/can/usb/f81604.c:f81604_handle_can_bus_errors() {
    ...
	if (priv->clear_flags)
		schedule_work(&priv->clear_reg_work);
    ...
}

On unbind or disconnect, f81604_disconnect() does this:

	unregister_netdev(priv->netdev[i]);
	free_candev(priv->netdev[i]);

Nothing cancels or flushes clear_reg_work again before free_candev().
f81604_clear_reg_work() gets priv via container_of() and then calls
f81604_sja1000_read() and f81604_sja1000_write() on it.

Would this be a use-after-free of the netdev private data and of the
embedded work_struct?

This ordering has been there since the driver was added in 88da17436973.
After 7299b1b39a25, every 9-byte interrupt failed the < 12 check, so this
schedule_work() call was effectively never reached. With this patch,
valid interrupts are handled again.

Could f81604_close() kill the anchored URBs before cancel_work_sync()?
Alternatively, could f81604_disconnect() cancel the work again before
free_candev()?

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]