Re: [PATCH net-next 29/37] can: f81604: f81604_close(): fix use-after-free on disconnect

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] can: f81604: TOCTOU double-free in echo_skb array during f81604_close()
--

commit 169687a41025adccdc4855da35895fe6f761e8b9
Author: Ji-Ze Hong (Peter Hong) <peter_hong@fintek.com.tw>

can: f81604: f81604_close(): fix use-after-free on disconnect

This commit fixes a use-after-free by unregistering URBs before cancelling
the clear_reg_work in f81604_close().

Link: https://patch.msgid.link/20260901-f81604-fix-v1-1-c55b5178f9f8@fintek.com.tw

> 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);
>  
>  	f81604_unregister_urbs(priv);

[Severity: High]
Can this ordering cause a double-free if an URB completion handler runs
concurrently with close_candev()?

When f81604_close() invokes close_candev() before unregistering in-flight
URBs, close_candev() calls can_flush_echo_skb() which iterates over
priv->echo_skb[] to free pending skbs locklessly:

drivers/net/can/dev/skb.c:can_flush_echo_skb() {
	...
	if (priv->echo_skb[i]) {
		kfree_skb(priv->echo_skb[i]);
		priv->echo_skb[i] = NULL;
	...
}

Concurrently, an active URB completion can trigger f81604_handle_tx() and
invoke can_get_echo_skb(). Because __can_get_echo_skb() also tests the skb
pointer locklessly:

drivers/net/can/dev/skb.c:__can_get_echo_skb() {
	...
	if (priv->echo_skb[idx]) {
		struct sk_buff *skb = priv->echo_skb[idx];
		...
		priv->echo_skb[idx] = NULL;
		...
		dev_consume_skb_any(skb);
	...
}

If both threads observe a non-NULL pointer for the same index, could this
result in the same skb being freed twice?

> +	cancel_work_sync(&priv->clear_reg_work);
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009134323.64064-1-mkl@pengutronix.de?part=29