Re: [PATCH] can: j1939: fix potential race condition in BAM segmentation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Remote-triggerable reference count leak of `j1939_session` due to un-cancelled `txtimer` when receiving duplicate or spoofed BAM data loopback frames.
--

commit d4773792dedead1d2a7d20d129ec0234fba6902b
Author: Alexander Hölzl <alexander.hoelzl@gmx.net>

can: j1939: fix potential race condition in BAM segmentation

This commit addresses a potential race condition in segmented messages using
the BAM protocol. It moves the TX timer scheduling so the timer is only
scheduled when the TX acknowledgment is received.

> diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
> index df93d57907da7..d00f0c158bb8e 100644
> --- a/net/can/j1939/transport.c
> +++ b/net/can/j1939/transport.c

[ ... ]

> @@ -1948,6 +1960,10 @@ static void j1939_xtp_rx_dat_one(struct j1939_session *session,
>  	} else if (remain) {
>  		if (!session->transmission)
>  			j1939_tp_set_rxtimeout(session, 750);
> +		else if (j1939_cb_is_broadcast(&session->skcb)) {
> +			j1939_tp_schedule_txtimer(session, J1939_TP_BAM_FRAME_SPACING_MS);

[Severity: High]
Does this unconditionally leak a j1939_session reference if a duplicate or
spoofed BAM loopback frame arrives while the timer is already active?

If a duplicate DAT loopback frame is received, the code detects it and falls
through to the remain = true block, calling j1939_tp_schedule_txtimer() again:

net/can/j1939/transport.c:j1939_tp_schedule_txtimer() {
	j1939_session_get(session);
	hrtimer_start(&session->txtimer, ms_to_ktime(msec),
		      HRTIMER_MODE_REL_SOFT);
}

Since j1939_tp_schedule_txtimer() takes a session reference without cancelling
any existing timer, and the timer callback will only fire once, won't the extra
references permanently leak and prevent the session from being freed?

> +			j1939_tp_set_rxtimeout(session, J1939_TP_BAM_ECHO_TIMEOUT_MS);
> +		}
>  	} else if (do_cts_eoma) {
>  		j1939_tp_set_rxtimeout(session, 1250);
>  		if (!session->transmission)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610090619.70626-1-alexander.hoelzl@gmx.net?part=1