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: 9d71dd0c7009 ("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>

diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index df93d57907da7e5cb9dc004d551a209fdb5aa05f..8a31cb23bc76d0abf6451e3958119d1237626a88 100644 (file)
--- a/net/can/j1939/transport.c
+++ b/net/can/j1939/transport.c
@@ -351,6 +351,18 @@ static void j1939_session_skb_drop_old(struct j1939_session *session)
        }
 }
 
+static bool j1939_address_is_local(struct j1939_priv *priv, u8 addr)
+{
+       bool local = false;
+
+       read_lock_bh(&priv->lock);
+       if (j1939_address_is_unicast(addr) && priv->ents[addr].nusers)
+               local = true;
+       read_unlock_bh(&priv->lock);
+
+       return local;
+}
+
 void j1939_session_skb_queue(struct j1939_session *session,
                             struct sk_buff *skb)
 {
@@ -359,8 +371,7 @@ void j1939_session_skb_queue(struct j1939_session *session,
 
        j1939_ac_fixup(priv, skb);
 
-       if (j1939_address_is_unicast(skcb->addr.da) &&
-           priv->ents[skcb->addr.da].nusers)
+       if (j1939_address_is_local(priv, skcb->addr.da))
                skcb->flags |= J1939_ECU_LOCAL_DST;
 
        skcb->flags |= J1939_ECU_LOCAL_SRC;
@@ -2038,8 +2049,7 @@ struct j1939_session *j1939_tp_send(struct j1939_priv *priv,
                return ERR_PTR(ret);
 
        /* fix DST flags, it may be used there soon */
-       if (j1939_address_is_unicast(skcb->addr.da) &&
-           priv->ents[skcb->addr.da].nusers)
+       if (j1939_address_is_local(priv, skcb->addr.da))
                skcb->flags |= J1939_ECU_LOCAL_DST;
 
        /* src is always local, I'm sending ... */