[PATCH] can: j1939: Use hrtimer_cancel in j1939_cancel_active_session

[PATCH] can: j1939: Use hrtimer_cancel in j1939_cancel_active_session

[Dudu Lu]

j1939_cancel_active_session() uses hrtimer_try_to_cancel() for both
txtimer and rxtimer. When hrtimer_try_to_cancel() returns -1, it means
the timer callback is currently executing. In this case, the function
neither cancels the timer nor drops the session reference via
j1939_session_put(). The session is then deactivated while the timer
callback may still be running, leading to a potential use-after-free if
the callback accesses the session after it has been cleaned up.

Replace hrtimer_try_to_cancel() with hrtimer_cancel() which will wait
for an in-progress callback to complete before returning, ensuring the
timer is fully stopped before the session is deactivated.

Note: This changes the function to potentially sleep (hrtimer_cancel
waits for the callback). The function is called with
active_session_list_lock held (a spinlock), so an alternative approach
would be to handle the -1 return from hrtimer_try_to_cancel() without
blocking. However, if the lock can be converted or the cancel moved
outside the lock, hrtimer_cancel() is the cleaner fix.

Signed-off-by: Dudu Lu <phx0fer@gmail.com>
---
 net/can/j1939/transport.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index df93d57907da..996d4c2cbbb0 100644
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -2230,9 +2230,9 @@ int j1939_cancel_active_session(struct j1939_priv *priv, struct sock *sk)
 				 &priv->active_session_list,
 				 active_session_list_entry) {
 		if (!sk || sk == session->sk) {
-			if (hrtimer_try_to_cancel(&session->txtimer) == 1)
+			if (hrtimer_cancel(&session->txtimer))
 				j1939_session_put(session);
-			if (hrtimer_try_to_cancel(&session->rxtimer) == 1)
+			if (hrtimer_cancel(&session->rxtimer))
 				j1939_session_put(session);
 
 			session->err = ESHUTDOWN;
-- 
2.39.3 (Apple Git-145)

Re: [PATCH] can: j1939: Use hrtimer_cancel in j1939_cancel_active_session

[Oleksij Rempel]

Hi Dudu,

On Mon, Apr 13, 2026 at 04:45:48PM +0800, Dudu Lu wrote:
> j1939_cancel_active_session() uses hrtimer_try_to_cancel() for both
> txtimer and rxtimer. When hrtimer_try_to_cancel() returns -1, it means
> the timer callback is currently executing. In this case, the function
> neither cancels the timer nor drops the session reference via
> j1939_session_put(). The session is then deactivated while the timer
> callback may still be running, leading to a potential use-after-free if
> the callback accesses the session after it has been cleaned up.
> 
> Replace hrtimer_try_to_cancel() with hrtimer_cancel() which will wait
> for an in-progress callback to complete before returning, ensuring the
> timer is fully stopped before the session is deactivated.
> 
> Note: This changes the function to potentially sleep (hrtimer_cancel
> waits for the callback). The function is called with
> active_session_list_lock held (a spinlock), so an alternative approach
> would be to handle the -1 return from hrtimer_try_to_cancel() without
> blocking. However, if the lock can be converted or the cancel moved
> outside the lock, hrtimer_cancel() is the cleaner fix.

Is it real or academical issue? Are there some bug report or exploit?

Best regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |