* [PATCH] can: usb: f81604: fix use-after-free on disconnect
@ 2026-09-01 14:27 ` Ji-Ze Hong via B4 Relay
0 siblings, 0 replies; 3+ messages in thread
From: Ji-Ze Hong via B4 Relay @ 2026-09-01 14:27 UTC (permalink / raw)
To: Marc Kleine-Budde, Vincent Mailhol
Cc: linux-can, linux-kernel, stable, Ji-Ze Hong (Peter Hong)
From: "Ji-Ze Hong (Peter Hong)" <peter_hong@fintek.com.tw>
Unregister the URBs before cancelling clear_reg_work to prevent an URB
completion handler from scheduling work after cancel_work_sync() has
returned.
Otherwise the pending work may run after the netdev and private data are
freed, resulting in a use-after-free.
Suggested-by: Marc Kleine-Budde <mkl@pengutronix.de>
Closes: https://lore.kernel.org/all/20260824133009.CF85A1F00A3A@smtp.kernel.org/
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 4c147b9d6d69..bb155b876924 100644
--- a/drivers/net/can/usb/f81604.c
+++ b/drivers/net/can/usb/f81604.c
@@ -1075,10 +1075,10 @@ static int f81604_close(struct net_device *netdev)
f81604_set_reset_mode(priv);
netif_stop_queue(netdev);
- cancel_work_sync(&priv->clear_reg_work);
close_candev(netdev);
f81604_unregister_urbs(priv);
+ cancel_work_sync(&priv->clear_reg_work);
return 0;
}
---
base-commit: 475cb4ea138f4b0e7f43ff79220f4013a6a840b8
change-id: 20260901-f81604-fix-9400ea120256
Best regards,
--
Ji-Ze Hong (Peter Hong) <peter_hong@fintek.com.tw>
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] can: usb: f81604: fix use-after-free on disconnect
2026-09-01 14:27 ` Ji-Ze Hong via B4 Relay
(?)
@ 2026-09-01 14:43 ` sashiko-bot
-1 siblings, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-01 14:43 UTC (permalink / raw)
To: Ji-Ze Hong (Peter Hong)
Cc: Oliver Hartkopp, Oleksij Rempel, Vincent Mailhol, linux-can,
Marc Kleine-Budde
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] Incorrect teardown order in `f81604_close()` calls `close_candev()` before killing URBs, leaving the device vulnerable to `restart_work` Use-After-Free and `echo_skb` Double-Free.
- [High] TX URBs are not anchored, causing them to leak and trigger a Use-After-Free on device disconnect.
- [High] Duplicate URB submission and memory leak during CAN bus-off recovery.
- [High] Pending `clear_reg_work` is leaked if `f81604_open()` fails, leading to a use-after-free upon device disconnect.
--
commit a9d42819f6aa6bc14b0da3547f398a4d371e4aab
Author: Ji-Ze Hong (Peter Hong) <peter_hong@fintek.com.tw>
Subject: can: usb: f81604: fix use-after-free on disconnect
This commit unregisters the URBs before cancelling clear_reg_work. This
prevents an URB completion handler from scheduling work after
cancel_work_sync() has returned, which would otherwise result in a
use-after-free.
Closes: https://lore.kernel.org/all/20260824133009.CF85A1F00A3A@smtp.kernel.org/
> diff --git a/drivers/net/can/usb/f81604.c b/drivers/net/can/usb/f81604.c
> index 4c147b9d6d690..bb155b8769245 100644
> --- a/drivers/net/can/usb/f81604.c
> +++ b/drivers/net/can/usb/f81604.c
> @@ -1075,10 +1075,10 @@ static int f81604_close(struct net_device *netdev)
> f81604_set_reset_mode(priv);
>
> netif_stop_queue(netdev);
> - cancel_work_sync(&priv->clear_reg_work);
> close_candev(netdev);
[Severity: High]
This isn't a bug introduced by this patch, but close_candev() is executed
while URBs are still active and capable of triggering concurrent callbacks.
Can this order result in a use-after-free or double-free?
If an active URB completes concurrently, could it trigger can_bus_off() (which
reschedules restart_work AFTER it was cancelled in close_candev()) or access
an echo_skb that is currently being flushed?
>
> f81604_unregister_urbs(priv);
> + cancel_work_sync(&priv->clear_reg_work);
>
> return 0;
> }
[Severity: High]
This is a pre-existing issue, but looking at f81604_start_xmit():
drivers/net/can/usb/f81604.c:f81604_start_xmit() {
[ ... ]
write_urb = usb_alloc_urb(0, GFP_ATOMIC);
if (!write_urb)
goto nomem_urb;
[ ... ]
ret = usb_submit_urb(write_urb, GFP_ATOMIC);
[ ... ]
}
The TX URB is created here but is never anchored to priv->urbs_anchor.
When the device is disconnected, f81604_disconnect() frees the netdev. Since
the TX URB is unanchored, f81604_close() does not kill it.
If the uncancelled TX URB completes asynchronously later, would it access the
freed netdev pointer in f81604_write_bulk_callback()?
drivers/net/can/usb/f81604.c:f81604_write_bulk_callback() {
struct net_device *netdev = urb->context;
if (!netif_device_present(netdev))
return;
[ ... ]
[Severity: High]
This is a pre-existing issue, but I noticed a potential leak during CAN
bus-off recovery:
drivers/net/can/usb/f81604.c:f81604_set_mode() {
switch (mode) {
case CAN_MODE_START:
ret = f81604_start(netdev);
[ ... ]
}
When the device enters CAN_STATE_BUS_OFF, the core schedules bus-off recovery
which invokes f81604_set_mode(..., CAN_MODE_START).
This calls f81604_start() which unconditionally registers URBs:
drivers/net/can/usb/f81604.c:f81604_start() {
[ ... ]
ret = f81604_register_urbs(priv);
if (ret)
return ret;
[ ... ]
}
Does this unconditionally allocate and submit a new set of RX and INT URBs
without killing or unregistering the previously active URBs?
[Severity: High]
This is a pre-existing issue, but there appears to be a leaked work struct if
f81604_open() fails:
drivers/net/can/usb/f81604.c:f81604_start() {
[ ... ]
ret = f81604_register_urbs(priv);
if (ret)
return ret;
ret = f81604_set_normal_mode(priv);
if (ret) {
f81604_unregister_urbs(priv);
return ret;
}
[ ... ]
}
If f81604_set_normal_mode() fails, the error path unregisters the URBs, but
does not cancel priv->clear_reg_work (which might have been scheduled if an
INT URB completed immediately).
Because the interface was never UP, ndo_stop (f81604_close()) is not called
when the device is unplugged:
drivers/net/can/usb/f81604.c:f81604_disconnect() {
[ ... ]
unregister_netdev(priv->netdev[i]);
free_candev(priv->netdev[i]);
[ ... ]
}
Will this cause a use-after-free when the pending clear_reg_work executes
and accesses the freed priv struct?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260901-f81604-fix-v1-1-c55b5178f9f8@fintek.com.tw?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread