Re: [PATCH v2] 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] Improper teardown order in `f81604_close()` allows asynchronous work to be scheduled after `cancel_work_sync`, leading to a Use-After-Free on device disconnect.
--

--- Patch [1]: [PATCH v2] can: usb: f81604: fix struct f81604_int_data size mismatch ---
commit 975e1f8f6c4a34bb9458d846d2ded822cbc82f04
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
so its size matches the actual 9-byte hardware data size. This prevents
valid interrupt messages from being incorrectly discarded as short URBs.

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 by fixing the struct size and enabling
normal interrupt processing, this patch actively unmasks a use-after-free
on device disconnect.

Looking at f81604_close(), cancel_work_sync() is called before the URBs
are unregistered:

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

    f81604_unregister_urbs(priv);
    ...
}

If an interrupt URB completes concurrently with a device disconnect,
could the URB completion handler schedule the work again after
cancel_work_sync() has already completed?

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

If so, f81604_unregister_urbs() will wait for the URB callback to finish
but leave the newly scheduled work pending. 

When the disconnect path returns and frees the netdev and its embedded
priv struct, will the delayed work execute and dereference the freed
pointer in f81604_clear_reg_work()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824-f81604-fix-v2-1-fc9be5581394@fintek.com.tw?part=1