Skip to content

Commit e4e8af6

Browse files
sfu2marckleinebudde
authored andcommitted
can: j1939: fix lockless local-destination check
j1939_priv.ents[].nusers is documented as protected by priv->lock, and its updates already happen under that lock. j1939_can_recv() also reads it under read_lock_bh(). However, j1939_session_skb_queue() and j1939_tp_send() still read priv->ents[da].nusers without taking the lock. Those transport-side checks decide whether to set J1939_ECU_LOCAL_DST, so they can race with j1939_local_ecu_get() and j1939_local_ecu_put() while userspace is binding or releasing sockets concurrently with TP traffic. This can misclassify TP/ETP sessions as local or remote and take the wrong transport path. Fix both transport paths by routing the destination-locality check through a helper that reads ents[].nusers under read_lock_bh(&priv->lock). Fixes: 9d71dd0 ("can: add support of SAE J1939 protocol") Signed-off-by: Shuhao Fu <sfual@cse.ust.hk> Tested-by: Oleksij Rempel <o.rempel@pengutronix.de> Acked-by: Oleksij Rempel <o.rempel@pengutronix.de> Link: https://patch.msgid.link/20260419140614.GA4041240@chcpu16 Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
1 parent 1e5185c commit e4e8af6

1 file changed

Lines changed: 14 additions & 4 deletions

File tree

net/can/j1939/transport.c

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,18 @@ static void j1939_session_skb_drop_old(struct j1939_session *session)
351351
}
352352
}
353353

354+
static bool j1939_address_is_local(struct j1939_priv *priv, u8 addr)
355+
{
356+
bool local = false;
357+
358+
read_lock_bh(&priv->lock);
359+
if (j1939_address_is_unicast(addr) && priv->ents[addr].nusers)
360+
local = true;
361+
read_unlock_bh(&priv->lock);
362+
363+
return local;
364+
}
365+
354366
void j1939_session_skb_queue(struct j1939_session *session,
355367
struct sk_buff *skb)
356368
{
@@ -359,8 +371,7 @@ void j1939_session_skb_queue(struct j1939_session *session,
359371

360372
j1939_ac_fixup(priv, skb);
361373

362-
if (j1939_address_is_unicast(skcb->addr.da) &&
363-
priv->ents[skcb->addr.da].nusers)
374+
if (j1939_address_is_local(priv, skcb->addr.da))
364375
skcb->flags |= J1939_ECU_LOCAL_DST;
365376

366377
skcb->flags |= J1939_ECU_LOCAL_SRC;
@@ -2038,8 +2049,7 @@ struct j1939_session *j1939_tp_send(struct j1939_priv *priv,
20382049
return ERR_PTR(ret);
20392050

20402051
/* fix DST flags, it may be used there soon */
2041-
if (j1939_address_is_unicast(skcb->addr.da) &&
2042-
priv->ents[skcb->addr.da].nusers)
2052+
if (j1939_address_is_local(priv, skcb->addr.da))
20432053
skcb->flags |= J1939_ECU_LOCAL_DST;
20442054

20452055
/* src is always local, I'm sending ... */

0 commit comments

Comments
 (0)