All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oliver Hartkopp <socketcan@hartkopp.net>
To: stable@vger.kernel.org
Cc: Oliver Hartkopp <socketcan@hartkopp.net>,
	Marc Kleine-Budde <mkl@pengutronix.de>
Subject: [PATCH 5.10.y 8/9] can: bcm: fix data race on rx_stamp/rx_ifindex in bcm_rx_handler()
Date: Fri, 24 Jul 2026 12:13:41 +0200	[thread overview]
Message-ID: <20260724101342.16000-8-socketcan@hartkopp.net> (raw)
In-Reply-To: <20260724101342.16000-1-socketcan@hartkopp.net>

commit 58fd6cbc8541216af1d7ed272ea7ac2b66d50fd8 upstream.

For an rx op subscribed on all interfaces (ifindex == 0), the same op
is registered once in the shared per-netns wildcard filter list, so
bcm_rx_handler() can run concurrently on different CPUs for frames
arriving on different net devices.

op->rx_stamp and op->rx_ifindex were written before bcm_rx_update_lock was
taken, allowing concurrent writers to race each other - including a torn
store of the 64-bit rx_stamp on 32-bit platforms.

Beyond a torn store bcm_send_to_user() must report the timestamp/ifindex
of the very same frame whose content it is delivering. So the assignment
is placed in the same unbroken bcm_rx_update_lock section as the content
comparison.

As a side effect, the RTR-request frame feature (which never reach
bcm_send_to_user()) no longer updates rx_stamp/rx_ifindex, since only
the notification path needs them.

Fixes: ffd980f976e7 ("[CAN]: Add broadcast manager (bcm) protocol")
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/linux-can/20260707145135.5BC831F00A3A@smtp.kernel.org/
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://patch.msgid.link/20260714-bcm_fixes-v15-10-562f7e3e42da@hartkopp.net
Cc: stable@kernel.org
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 net/can/bcm.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index c28256c..95eb791 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -756,11 +756,6 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
 	/* disable timeout */
 	hrtimer_cancel(&op->timer);
 
-	/* save rx timestamp */
-	op->rx_stamp = skb->tstamp;
-	/* save originator for recvfrom() */
-	op->rx_ifindex = skb->dev->ifindex;
-
 	/* op->flags/op->frames may be updated concurrently by bcm_rx_setup() */
 	spin_lock_bh(&op->bcm_rx_update_lock);
 
@@ -784,6 +779,14 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
 	 */
 	bcm_update_rx_stats(op);
 
+	/* save rx timestamp and originator for recvfrom() under lock.
+	 * For an op subscribed on all interfaces (ifindex == 0)
+	 * bcm_rx_handler() can run concurrently on different CPUs so
+	 * the CAN content and the meta data must be bundled correctly.
+	 */
+	op->rx_stamp = skb->tstamp;
+	op->rx_ifindex = skb->dev->ifindex;
+
 	if (op->flags & RX_FILTER_ID) {
 		/* the easiest case */
 		bcm_rx_update_and_send(op, op->last_frames, rxframe);
-- 
2.43.0


  parent reply	other threads:[~2026-07-24 10:14 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-20 15:12 FAILED: patch "[PATCH] can: bcm: defer rx_op deallocation to workqueue to fix" failed to apply to 5.10-stable tree gregkh
2026-07-24  8:35 ` [PATCH 1/9] can: bcm: defer rx_op deallocation to workqueue to fix thrtimer UAF Oliver Hartkopp
2026-07-24 10:13 ` [PATCH 5.10.y " Oliver Hartkopp
2026-07-24 10:13   ` [PATCH 5.10.y 2/9] can: bcm: add locking when updating filter and timer values Oliver Hartkopp
2026-07-24 10:13   ` [PATCH 5.10.y 3/9] can: bcm: fix CAN frame rx/tx statistics Oliver Hartkopp
2026-07-24 10:13   ` [PATCH 5.10.y 4/9] can: bcm: extend bcm_tx_lock usage for data and timer updates Oliver Hartkopp
2026-07-24 10:13   ` [PATCH 5.10.y 5/9] can: bcm: validate frame length in bcm_rx_setup() for RTR replies Oliver Hartkopp
2026-07-24 10:13   ` [PATCH 5.10.y 6/9] can: bcm: add missing device refcount for CAN filter removal Oliver Hartkopp
2026-07-24 10:13   ` [PATCH 5.10.y 7/9] can: bcm: fix stale rx/tx ops after device removal Oliver Hartkopp
2026-07-24 10:13   ` Oliver Hartkopp [this message]
2026-07-24 10:13   ` [PATCH 5.10.y 9/9] can: bcm: track a single source interface for ANYDEV timeout/throttle ops Oliver Hartkopp
2026-07-24 10:38   ` [PATCH 5.10.y 1/9] can: bcm: defer rx_op deallocation to workqueue to fix thrtimer UAF Greg KH
2026-07-24 10:43     ` Oliver Hartkopp
2026-07-28 16:09   ` Sasha Levin
2026-07-28 16:45     ` Oliver Hartkopp

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=20260724101342.16000-8-socketcan@hartkopp.net \
    --to=socketcan@hartkopp.net \
    --cc=mkl@pengutronix.de \
    --cc=stable@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.