diff mbox series

[5.10,052/593] can: isotp: isotp_release(): omit unintended hrtimer restart on socket release

Message ID 20210712060848.848876578@linuxfoundation.org
State New
Headers show
Series None | expand

Commit Message

Greg KH July 12, 2021, 6:03 a.m. UTC 
From: Oliver Hartkopp <socketcan@hartkopp.net>

commit 14a4696bc3118ba49da28f79280e1d55603aa737 upstream.

When closing the isotp socket, the potentially running hrtimers are
canceled before removing the subscription for CAN identifiers via
can_rx_unregister().

This may lead to an unintended (re)start of a hrtimer in
isotp_rcv_cf() and isotp_rcv_fc() in the case that a CAN frame is
received by isotp_rcv() while the subscription removal is processed.

However, isotp_rcv() is called under RCU protection, so after calling
can_rx_unregister, we may call synchronize_rcu in order to wait for
any RCU read-side critical sections to finish. This prevents the
reception of CAN frames after hrtimer_cancel() and therefore the
unintended (re)start of the hrtimers.

Link: https://lore.kernel.org/r/20210618173713.2296-1-socketcan@hartkopp.net
Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol")
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 net/can/isotp.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
diff mbox series

Patch

--- a/net/can/isotp.c
+++ b/net/can/isotp.c
@@ -1023,9 +1023,6 @@  static int isotp_release(struct socket *
 
 	lock_sock(sk);
 
-	hrtimer_cancel(&so->txtimer);
-	hrtimer_cancel(&so->rxtimer);
-
 	/* remove current filters & unregister */
 	if (so->bound) {
 		if (so->ifindex) {
@@ -1037,10 +1034,14 @@  static int isotp_release(struct socket *
 						  SINGLE_MASK(so->rxid),
 						  isotp_rcv, sk);
 				dev_put(dev);
+				synchronize_rcu();
 			}
 		}
 	}
 
+	hrtimer_cancel(&so->txtimer);
+	hrtimer_cancel(&so->rxtimer);
+
 	so->ifindex = 0;
 	so->bound = 0;