can: isotp: use unconditional synchronize_rcu() in isotp_release()

isotp_notify() unregisters the (RCU) CAN filters via can_rx_unregister()
and clears so->bound without waiting for a grace period. isotp_release()
uses so->bound to decide whether it needs to call synchronize_rcu()
before cancelling so->rxtimer, so when NETDEV_UNREGISTER runs first it
skips that synchronize_rcu() and can cancel the timer while an
in-flight isotp_rcv() is still executing and about to re-arm it via
isotp_send_fc(), leading to a use-after-free timer callback on the
freed socket.

sakisho-bot remarked a problem with rtnl_lock held in isotp_notify(),
therefore make isotp_release() always call synchronize_rcu() before
cancelling the timers, regardless of so->bound. This still closes the
original race (isotp_notify() clearing so->bound without waiting for
in-flight isotp_rcv() callers before isotp_release() cancels the RX
timer) without adding any RCU wait to the netdevice notifier path.

Fixes: 14a4696bc311 ("can: isotp: isotp_release(): omit unintended hrtimer restart on socket release")
Closes: https://lore.kernel.org/linux-can/20260707085210.6B6C01F000E9@smtp.kernel.org/
Reported-by: Nico Yip <zdi-disclosures@trendmicro.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://patch.msgid.link/20260712-isotp-fixes-v10-1-793a1b1ce17f@hartkopp.net
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

diff --git a/net/can/isotp.c b/net/can/isotp.c
index c48b4a818297e2a1348a2b64016d0f4ff613e683..d30937345bcdcec07507a91805431ac6b6ea9cdc 100644 (file)
--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1237,11 +1237,18 @@ static int isotp_release(struct socket *sock)
                                                  SINGLE_MASK(so->txid),
                                                  isotp_rcv_echo, sk);
                                dev_put(dev);
-                               synchronize_rcu();
                        }
                }
        }
 
+       /* Always wait for a grace period before touching the timers below.
+        * A concurrent NETDEV_UNREGISTER may have already unregistered our
+        * filters and cleared so->bound in isotp_notify() without waiting
+        * for in-flight isotp_rcv() callers to finish, so this call must not
+        * be skipped just because so->bound is already 0 here.
+        */
+       synchronize_rcu();
+
        hrtimer_cancel(&so->txfrtimer);
        hrtimer_cancel(&so->txtimer);
        hrtimer_cancel(&so->rxtimer);