Netdev List
 help / color / mirror / Atom feed
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: netdev@vger.kernel.org
Cc: davem@davemloft.net, kuba@kernel.org, linux-can@vger.kernel.org,
	kernel@pengutronix.de, Shuhao Fu <sfual@cse.ust.hk>,
	Oleksij Rempel <o.rempel@pengutronix.de>,
	Marc Kleine-Budde <mkl@pengutronix.de>
Subject: [PATCH net 04/19] can: j1939: fix lockless local-destination check
Date: Thu, 16 Jul 2026 17:47:29 +0200	[thread overview]
Message-ID: <20260716155528.809908-5-mkl@pengutronix.de> (raw)
In-Reply-To: <20260716155528.809908-1-mkl@pengutronix.de>

From: Shuhao Fu <sfual@cse.ust.hk>

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>
---
 net/can/j1939/transport.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/net/can/j1939/transport.c b/net/can/j1939/transport.c
index df93d57907da..8a31cb23bc76 100644
--- 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 ... */
-- 
2.53.0


  parent reply	other threads:[~2026-07-16 15:56 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-16 15:47 [PATCH net 0/19] pull-request: can 2026-07-16 Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 01/19] can: vxcan: Kconfig: fix description stating no local echo provided Marc Kleine-Budde
2026-07-17  9:50   ` patchwork-bot+netdevbpf
2026-07-16 15:47 ` [PATCH net 02/19] can: esd_usb: kill anchored URBs before freeing netdevs Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 03/19] can: raw: add locking for raw flags bitfield Marc Kleine-Budde
2026-07-16 15:47 ` Marc Kleine-Budde [this message]
2026-07-16 15:47 ` [PATCH net 05/19] can: peak: Modification of references to email accounts being deleted Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 06/19] can: bcm: defer rx_op deallocation to workqueue to fix thrtimer UAF Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 07/19] can: bcm: fix lockless bound/ifindex race and silent RX_SETUP failure Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 08/19] can: bcm: add locking when updating filter and timer values Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 09/19] can: bcm: fix CAN frame rx/tx statistics Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 10/19] can: bcm: add missing rcu list annotations and operations Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 11/19] can: bcm: extend bcm_tx_lock usage for data and timer updates Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 12/19] can: bcm: validate frame length in bcm_rx_setup() for RTR replies Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 13/19] can: bcm: add missing device refcount for CAN filter removal Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 14/19] can: bcm: fix stale rx/tx ops after device removal Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 15/19] can: bcm: fix data race on rx_stamp/rx_ifindex in bcm_rx_handler() Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 16/19] can: bcm: track a single source interface for ANYDEV timeout/throttle ops Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 17/19] can: isotp: use unconditional synchronize_rcu() in isotp_release() Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 18/19] can: isotp: fix use-after-free race with concurrent NETDEV_UNREGISTER Marc Kleine-Budde
2026-07-16 15:47 ` [PATCH net 19/19] can: isotp: serialize TX state transitions under so->rx_lock Marc Kleine-Budde

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260716155528.809908-5-mkl@pengutronix.de \
    --to=mkl@pengutronix.de \
    --cc=davem@davemloft.net \
    --cc=kernel@pengutronix.de \
    --cc=kuba@kernel.org \
    --cc=linux-can@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=sfual@cse.ust.hk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox