From: sashiko-bot@kernel.org
To: "Oliver Hartkopp" <socketcan@hartkopp.net>
Cc: linux-can@vger.kernel.org,
"Marc Kleine-Budde" <mkl@pengutronix.de>,
"Vincent Mailhol" <mailhol@kernel.org>,
"Oleksij Rempel" <o.rempel@pengutronix.de>,
"Oliver Hartkopp" <socketcan@hartkopp.net>
Subject: Re: [PATCH v12 04/11] can: bcm: fix CAN frame rx/tx statistics
Date: Thu, 09 Jul 2026 20:35:50 +0000 [thread overview]
Message-ID: <20260709203551.1A1331F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260709-bcm_fixes-v12-4-61c06291bf1c@hartkopp.net>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Unsynchronized modification of rx_stamp and rx_ifindex allows concurrent frames to corrupt each other's metadata.
--
--- Patch [4]: [PATCH v12 04/11] can: bcm: fix CAN frame rx/tx statistics ---
commit f0713957841f9a5510dbab52a7bc1b9ca16a367e
Author: Oliver Hartkopp <socketcan@hartkopp.net>
can: bcm: fix CAN frame rx/tx statistics
This commit addresses a KCSAN-detected data race in bcm_rx_handler() by
converting rx/tx statistics to atomic operations. It also simplifies
overflow handling and resolves a race condition where frames_filtered
could become larger than frames_abs.
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
[ ... ]
> @@ -748,24 +767,30 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
> op->rx_stamp = skb->tstamp;
> /* save originator for recvfrom() */
> op->rx_ifindex = skb->dev->ifindex;
[Severity: High]
This is a pre-existing issue, but does this unsynchronized modification
allow concurrent frames to corrupt each other's metadata?
When a wildcard (ANYDEV) CAN BCM receive operation is configured,
multiple interfaces might receive matching packets simultaneously. Because
op->rx_stamp and op->rx_ifindex are updated outside of
bcm_rx_update_lock, could different CPUs concurrently overwrite these
fields?
If so, this might result in userspace receiving packets with mismatched
timestamps and source interface indices, potentially causing logic errors
in routing or packet processing.
> - /* update statistics */
> - op->frames_abs++;
>
> - /* snapshot the flag under lock: op->flags/op->frames may be updated
> - * concurrently by bcm_rx_setup().
> - */
> + /* op->flags/op->frames may be updated concurrently by bcm_rx_setup() */
> spin_lock_bh(&op->bcm_rx_update_lock);
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-bcm_fixes-v12-0-61c06291bf1c@hartkopp.net?part=4
next prev parent reply other threads:[~2026-07-09 20:35 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-09 20:22 [PATCH v12 00/11] can: bcm: collected fixes - final7 \o/ \o/ Oliver Hartkopp via B4 Relay
2026-07-09 20:22 ` [PATCH v12 01/11] can: bcm: defer rx_op deallocation to workqueue to fix thrtimer UAF Oliver Hartkopp via B4 Relay
2026-07-09 20:40 ` sashiko-bot
2026-07-09 20:22 ` [PATCH v12 02/11] can: bcm: fix lockless bound/ifindex race and silent RX_SETUP failure Oliver Hartkopp via B4 Relay
2026-07-10 0:40 ` sashiko-bot
2026-07-09 20:22 ` [PATCH v12 03/11] can: bcm: add locking when updating filter and timer values Oliver Hartkopp via B4 Relay
2026-07-09 20:38 ` sashiko-bot
2026-07-09 20:22 ` [PATCH v12 04/11] can: bcm: fix CAN frame rx/tx statistics Oliver Hartkopp via B4 Relay
2026-07-09 20:35 ` sashiko-bot [this message]
2026-07-09 20:22 ` [PATCH v12 05/11] can: bcm: add missing rcu list annotations and operations Oliver Hartkopp via B4 Relay
2026-07-09 20:35 ` sashiko-bot
2026-07-09 20:22 ` [PATCH v12 06/11] can: bcm: extend bcm_tx_lock usage for data and timer updates Oliver Hartkopp via B4 Relay
2026-07-09 20:22 ` [PATCH v12 07/11] can: bcm: validate frame length in bcm_rx_setup() for RTR replies Oliver Hartkopp via B4 Relay
2026-07-09 20:22 ` [PATCH v12 08/11] can: bcm: add missing device refcount for CAN filter removal Oliver Hartkopp via B4 Relay
2026-07-09 20:35 ` sashiko-bot
2026-07-09 20:22 ` [PATCH v12 09/11] can: bcm: fix stale rx/tx ops after device removal Oliver Hartkopp via B4 Relay
2026-07-09 20:40 ` sashiko-bot
2026-07-09 20:22 ` [PATCH v12 10/11] can: bcm: fix data race on rx_stamp/rx_ifindex in bcm_rx_handler() Oliver Hartkopp via B4 Relay
2026-07-09 20:44 ` sashiko-bot
2026-07-09 20:22 ` [PATCH v12 11/11] can: bcm: track a single source interface for ANYDEV timeout/throttle ops Oliver Hartkopp via B4 Relay
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=20260709203551.1A1331F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=linux-can@vger.kernel.org \
--cc=mailhol@kernel.org \
--cc=mkl@pengutronix.de \
--cc=o.rempel@pengutronix.de \
--cc=sashiko-reviews@lists.linux.dev \
--cc=socketcan@hartkopp.net \
/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