Re: [PATCH net 3/3] usb: f81604: fix struct f81604_int_data size mismatch

[Marc Kleine-Budde <mkl@pengutronix.de>]

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

On 01.10.2026 17:14:25, Marc Kleine-Budde 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>
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Tested-by: Drew Willey <dwilley@google.com>
> Link: https://patch.msgid.link/20260824-f81604-fix-v2-1-fc9be5581394@fintek.com.tw
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

The netdev bots have found a "high" severity, which is a preexisting
issue, but not clearly flagged as such.

https://netdev-ai.bots.linux.dev/sashiko/#inline-kq9vc8hku

| Consequence: use-after-free of the freed netdev private data and of the
| embedded work_struct, which can corrupt memory or oops in the workqueue.
|
| Triggering path: interface up with CAN bus errors, arbitration loss or
| overruns (so clear_flags gets set) -> driver unbind, rmmod or disconnect
| -> unregister_netdev() -> f81604_close() -> cancel_work_sync() returns
| -> an interrupt URB completion in flight before usb_kill_anchored_urbs()
| calls f81604_handle_can_bus_errors() -> schedule_work() -> close returns
| -> free_candev() -> f81604_clear_reg_work() runs on freed memory. I
| checked that nothing between line 1078 and the free in
| f81604_disconnect() drains this work: close_candev() and
| usb_kill_anchored_urbs() do not flush it, and disconnect only calls
| unregister_netdev() and free_candev().
|
| Reachability: the race window is short, and triggering it needs
| privileged unbind or physical disconnect while bus errors are occurring.
| So it rates High rather than Critical, though the consequence is memory
| corruption. Pre-existing: git blame attributes f81604_close() entirely
| to 88da17436973. This patch does not add the ordering, but it re-enables
| the only producer of the work, which had been dead code since
| 7299b1b39a25. The commit message neither claims to fix this nor depends
| on it.

We have a patch for it pending, but not included in the PR, to keep it
to the absolute minimum.

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