Re: [PATCH net v8 1/2] can: j1939: account for timer references across rearm

[Oleksij Rempel <o.rempel@pengutronix.de>]

Hi Zihan,

Thank you for your work.

To avoid duplicate work, please take a look at this series:
https://lore.kernel.org/all/a1dd9760-7815-400a-a39c-a1a03cf8262e@I-love.SAKURA.ne.jp/

and sync it with Tetsuo.

On Mon, Nov 09, 2026 at 01:16:23PM +0000, Zihan Xi wrote:
> Rearming a session timer updates the existing hrtimer rather than
> creating another callback. Taking a session reference for every start
> can therefore leave references behind when the timer is rearmed.
> 
> Track one session reference across each timer's queued and running
> lifetime. Apply the same accounting to the RX timer and synchronize
> rearming and cancellation with terminal state changes. Arm the simple-TX
> echo timeout only after a successful send, and keep terminal session
> transitions from leaving stale timer references or changing the result.
> 
> Fixes: 9d71dd0c7009 ("can: add support of SAE J1939 protocol")
> Cc: stable@vger.kernel.org
> Reported-by: Vega <vega@nebusec.ai>
> Link: https://bugtracker.nebusec.ai/f/11143
> Signed-off-by: Zihan Xi <zihanx@nebusec.ai>
> ---
>  net/can/j1939/j1939-priv.h |  11 +
>  net/can/j1939/transport.c  | 510 +++++++++++++++++++++++++++++++------
>  2 files changed, 438 insertions(+), 83 deletions(-)
> 
> diff --git a/net/can/j1939/j1939-priv.h b/net/can/j1939/j1939-priv.h
> index cf26352d1d8c..123622c82a98 100644
> --- a/net/can/j1939/j1939-priv.h
> +++ b/net/can/j1939/j1939-priv.h
> @@ -10,6 +10,7 @@
>  #define _J1939_PRIV_H_
>  
>  #include <linux/can/j1939.h>
> +#include <linux/spinlock.h>
>  #include <net/sock.h>
>  
>  /* Timeout to receive the abort signal over loop back. In case CAN
> @@ -290,6 +291,16 @@ struct j1939_session {
>  		unsigned int dpo;
>  	} pkt;
>  	struct hrtimer txtimer, rxtimer;
> +	unsigned int txtimer_cancelers; /* block starts while a cancel waits */
> +	spinlock_t txtimer_lock; /* Protects TX timer lifetime state. */
> +	/* One reference pins the complete queued/running timer lifetime. */
> +	bool txtimer_shutdown;
> +	bool txtimer_ref;
> +	spinlock_t rxtimer_lock; /* Protects RX timer lifetime state. */
> +	unsigned int rxtimer_cancelers;
> +	/* One reference pins the complete queued/running timer lifetime. */
> +	bool rxtimer_shutdown;
> +	bool rxtimer_ref;

If you add new variables, pleas use __guarded_by()
https://docs.kernel.org/dev-tools/context-analysis.html

Thank you!

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 |