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

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

[PS10 PETER HONG 洪繼澤]

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: 88da17436973 ("can: usb: f81604: add Fintek F81604 support")
Fixes: 7299b1b39a25 ("can: usb: f81604: handle short interrupt urb messages properly")
Cc: stable@vger.kernel.org
Signed-off-by: Ji-Ze Hong (Peter Hong) <peter_hong@fintek.com.tw>
---
 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;
--
2.43.0

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

[Greg KH]

On Mon, Aug 24, 2026 at 04:27:58PM +0800, PS10 PETER HONG 洪繼澤 wrote:
> 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: 88da17436973 ("can: usb: f81604: add Fintek F81604 support")
> Fixes: 7299b1b39a25 ("can: usb: f81604: handle short interrupt urb messages properly")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ji-Ze Hong (Peter Hong) <peter_hong@fintek.com.tw>

Nit, doesn't match the From: line :(

Also, the first Fixes: tag isn't correct, it's the second one that
matters.

And wasn't this reported by someone already:
	https://lore.kernel.org/r/A3834A07-5639-4779-844F-C5843DFC3928@dynetrex.com
?

And yes, this patch does look correct.

thanks,

greg k-h

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

[Dynetrex, Admin]

Hey Greg, 

This issue was reported by me about a month and a half ago, however, I did not have an environment setup for kernel development in order to submit a patch.

I appreciate Peter taking a look at this.

Kind Regards,
Alex

> On Aug 24, 2026, at 2:08 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> On Mon, Aug 24, 2026 at 04:27:58PM +0800, PS10 PETER HONG 洪繼澤 wrote:
>> 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: 88da17436973 ("can: usb: f81604: add Fintek F81604 support")
>> Fixes: 7299b1b39a25 ("can: usb: f81604: handle short interrupt urb messages properly")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Ji-Ze Hong (Peter Hong) <peter_hong@fintek.com.tw>
> 
> Nit, doesn't match the From: line :(
> 
> Also, the first Fixes: tag isn't correct, it's the second one that
> matters.
> 
> And wasn't this reported by someone already:
> 	https://lore.kernel.org/r/A3834A07-5639-4779-844F-C5843DFC3928@dynetrex.com
> ?
> 
> And yes, this patch does look correct.
> 
> thanks,
> 
> greg k-h

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

[Greg KH]

On Mon, Aug 24, 2026 at 02:14:26AM -0700, Dynetrex, Admin wrote:
> Hey Greg, 
> 
> This issue was reported by me about a month and a half ago, however, I did not have an environment setup for kernel development in order to submit a patch.

Great, a reported-by: line would be great to have added here to capture
that.

thanks,

greg k-h

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

[Marc Kleine-Budde]

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

On 24.08.2026 17:38:23, PS10 PETER HONG 洪繼澤 wrote:
> Greg KH 於 2026/8/24 下午 05:08 寫道:
> > On Mon, Aug 24, 2026 at 04:27:58PM +0800, PS10 PETER HONG 洪繼澤 wrote:
> >
> > Fixes: 88da17436973 ("can: usb: f81604: add Fintek F81604 support")
> > Fixes: 7299b1b39a25 ("can: usb: f81604: handle short interrupt urb messages properly")
> > Cc:stable@vger.kernel.org
> > Signed-off-by: Ji-Ze Hong (Peter Hong)<peter_hong@fintek.com.tw>
> > Nit, doesn't match the From: line :(
>
> Sorry, our company recently changed mail system, which now overrides
> the display name with the internal name.
>
> Could I resend the patch with the internal name in the Signed-off-by: line?

You can use the b4 "web submission" instead? It bypasses any mail system
completely.

| https://b4.docs.kernel.org/en/latest/contributor/send.html

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] can: usb: f81604: fix struct f81604_int_data size mismatch

[sashiko-bot]

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

Pre-existing issues:
- [High] Improper teardown ordering in f81604_close() causes a use-after-free of the driver's private data via clear_reg_work.
- [High] Unbounded memory and URB resource leak during CAN bus-off recovery and interface toggle.
- [High] Soft lockup (CPU Denial of Service) due to bypassed URB status checks in the interrupt callback.
--

--- Patch [1]: [PATCH] can: usb: f81604: fix struct f81604_int_data size mismatch ---
commit 53e24ec343d9d1cbf17bed31729c1ccebdbe512a
Author: PS10 PETER HONG 洪繼澤 <peter_hong@fintek.com.tw>

can: usb: f81604: fix struct f81604_int_data size mismatch

The struct f81604_int_data was padded to 12 bytes but hardware sends exactly
9 bytes, causing short URB checks to discard valid messages and resulting in
transmission failures. This commit removes the padding attribute so the struct
size matches the 9-byte hardware data length.

> 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;
>  
>  struct f81604_sff {
>  	__be16 id;

[Severity: High]
This isn't a bug introduced by this patch, but is the teardown ordering in
f81604_close() correct?

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

	f81604_unregister_urbs(priv);
...
}

Because f81604_unregister_urbs() is called after cancel_work_sync(), URBs are
still active. A concurrently executing interrupt URB callback can schedule the
work again via f81604_handle_can_bus_errors():

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

When the device is later disconnected, f81604_disconnect() frees the netdev
and its embedded priv structure. Will the pending clear_reg_work then execute
and access the freed priv pointer, leading to a memory corruption or crash?

[Severity: High]
This is a pre-existing issue, but does the driver leak URBs and their allocated
buffers during bus-off recovery and interface toggles?

When the CAN bus recovers from a bus-off condition, f81604_set_mode() calls
f81604_start() which registers new URBs:

drivers/net/can/usb/f81604.c:f81604_set_mode() {
	switch (mode) {
	case CAN_MODE_START:
		ret = f81604_start(netdev);
...
}

f81604_start() blindly calls f81604_register_urbs(), allocating and submitting
5 new URBs. The old URBs were never killed because can_bus_off() does not stop
them, seemingly leaking 5 URBs on every recovery.

Additionally, during f81604_close(), if an URB callback is executing
concurrently, it checks netif_device_present() instead of netif_running():

drivers/net/can/usb/f81604.c:f81604_read_bulk_callback() {
...
	if (!netif_device_present(netdev))
		return;
...
resubmit_urb:
	usb_anchor_urb(urb, &priv->urbs_anchor);
	ret = usb_submit_urb(urb, GFP_ATOMIC);
...
}

Will this cause the URB to be resubmitted even after ndo_stop has run, and when
the interface is brought back up, multiply the number of active URBs leading to
resource exhaustion?

[Severity: High]
This is a pre-existing issue, but can the short length check in the interrupt
callback cause a soft lockup?

In f81604_read_int_callback(), the short URB check is performed before the
status check:

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) {
...
}

When a USB hardware error occurs, the host controller completes the URB with an
error status and an actual_length of 0. Because 0 < 9, the code logs a warning
and jumps straight to resubmit_urb, bypassing the error-handling switch.

Will the host controller then immediately fail the resubmitted URB again,
triggering an infinite loop of hardware errors in softirq context and leading
to a CPU soft lockup?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824082758.9118-1-peter_hong@fintek.com.tw?part=1