All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Oliver Hartkopp" <socketcan@hartkopp.net>
Cc: "Marc Kleine-Budde" <mkl@pengutronix.de>,
	linux-can@vger.kernel.org,
	"Oliver Hartkopp" <socketcan@hartkopp.net>,
	"Oleksij Rempel" <o.rempel@pengutronix.de>,
	"Vincent Mailhol" <mailhol@kernel.org>
Subject: Re: [PATCH v4 4/7] can: bcm: fix CAN frame rx/tx statistics
Date: Tue, 07 Jul 2026 14:54:33 +0000	[thread overview]
Message-ID: <20260707145434.2410F1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260707-bcm_fixes-v4-4-aa87b8b9d33f@hartkopp.net>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing RCU memory barriers when inserting new bcm_op entries into rx_ops and tx_ops, and incorrect list traversal macro for tx_ops in a lockless reader.
--

--- Patch [4]: [PATCH v4 4/7] can: bcm: fix CAN frame rx/tx statistics ---
commit 55dc712979cc536660202ae53cf4b6529bfdf948
Author: Oliver Hartkopp <socketcan@hartkopp.net>

can: bcm: fix CAN frame rx/tx statistics

This commit fixes a data race detected by KCSAN in bcm_rx_handler by
using atomic types for frame statistics, and adds a lock-protected
helper for updates to avoid races during resets.

> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -229,10 +229,13 @@ static int bcm_proc_show(struct seq_file *m, void *v)
>  
>  	list_for_each_entry_rcu(op, &bo->rx_ops, list) {

[Severity: High]
This isn't a bug introduced by this patch, but there appears to be a
missing RCU barrier when traversing and modifying the bcm_op lists.

Can the lockless reader in bcm_proc_show() observe uninitialized memory
here?

While the reader correctly uses list_for_each_entry_rcu() for rx_ops, the
writer in bcm_rx_setup() adds new entries using the non-RCU list_add():

net/can/bcm.c:bcm_rx_setup() {
    ...
    list_add(&op->list, &bo->rx_ops);
    ...
}

Without list_add_rcu(), the CPU could reorder the initialization of the
bcm_op struct to occur after the node becomes visible in the list.

Similarly, does the tx_ops list traversal lack both writer and reader
RCU protection?

net/can/bcm.c:bcm_tx_setup() {
    ...
    list_add(&op->list, &bo->tx_ops);
    ...
}

And later in bcm_proc_show():

net/can/bcm.c:bcm_proc_show() {
    ...
    list_for_each_entry(op, &bo->tx_ops, list) {
    ...
}

Should these lists be updated to use list_add_rcu() and
list_for_each_entry_rcu() to ensure safe concurrent iteration via procfs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-bcm_fixes-v4-0-aa87b8b9d33f@hartkopp.net?part=4

  reply	other threads:[~2026-07-07 14:54 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-07 14:32 [PATCH v4 0/7] can: bcm: collected fixes Oliver Hartkopp via B4 Relay
2026-07-07 14:32 ` Oliver Hartkopp
2026-07-07 14:32 ` [PATCH v4 1/7] can: bcm: defer rx_op deallocation to workqueue to fix thrtimer UAF Oliver Hartkopp via B4 Relay
2026-07-07 14:32   ` Oliver Hartkopp
2026-07-07 14:47   ` sashiko-bot
2026-07-07 14:32 ` [PATCH v4 2/7] can: bcm: fix lockless bound/ifindex race and silent RX_SETUP failure Oliver Hartkopp via B4 Relay
2026-07-07 14:32   ` Oliver Hartkopp
2026-07-07 14:53   ` sashiko-bot
2026-07-07 14:32 ` [PATCH v4 3/7] can: bcm: add locking when updating filter and timer values Oliver Hartkopp via B4 Relay
2026-07-07 14:32   ` Oliver Hartkopp
2026-07-07 14:48   ` sashiko-bot
2026-07-07 14:32 ` [PATCH v4 4/7] can: bcm: fix CAN frame rx/tx statistics Oliver Hartkopp via B4 Relay
2026-07-07 14:32   ` Oliver Hartkopp
2026-07-07 14:54   ` sashiko-bot [this message]
2026-07-07 14:32 ` [PATCH v4 5/7] can: bcm: add missing rcu list annotations and operations Oliver Hartkopp via B4 Relay
2026-07-07 14:32   ` Oliver Hartkopp
2026-07-07 14:49   ` sashiko-bot
2026-07-07 14:32 ` [PATCH v4 6/7] can: bcm: extend bcm_tx_lock usage for data and timer updates Oliver Hartkopp via B4 Relay
2026-07-07 14:32   ` Oliver Hartkopp
2026-07-07 14:38   ` Marc Kleine-Budde
2026-07-07 14:48     ` Oliver Hartkopp
2026-07-07 14:32 ` [PATCH v4 7/7] can: bcm: validate frame length in bcm_rx_setup() for RTR replies Oliver Hartkopp via B4 Relay
2026-07-07 14:32   ` Oliver Hartkopp
2026-07-07 14:51   ` sashiko-bot

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=20260707145434.2410F1F000E9@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 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.