Re: [PATCH v3 5/7] can: bcm: add missing rcu list annotations and operations

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Lockless reads of 64-bit ktime_t values and TOCTOU races on op->count in bcm_tx_timeout_handler
--

--- Patch [5]: [PATCH v3 5/7] can: bcm: add missing rcu list annotations and operations ---
commit 8310d786e9cc8e49f889c95c47438e852dd137cc
Author: Oliver Hartkopp <socketcan@hartkopp.net>

can: bcm: add missing rcu list annotations and operations

This commit moves list_add_rcu() calls to the end of the setup code to cover
initial settings and fixes the missing list_del_rcu() before bcm_remove_op()
in bcm_release().

Link: https://lore.kernel.org/linux-can/20260610094654.A1FFE1F00893@smtp.kernel.org/

> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 0378e31ea10ed..f21d48d99aab6 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
[ ... ]
> @@ -1159,6 +1159,10 @@ static int bcm_tx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>  		op->flags |= TX_ANNOUNCE;
>  	}
>  
> +	/* add this bcm_op to the list of the tx_ops? */
> +	if (add_op_to_list)
> +		list_add_rcu(&op->list, &bo->tx_ops);
> +

[Severity: High]
This is a pre-existing issue, but I have a question about concurrent access
to the timer and count values in bcm_tx_timeout_handler().

When a userspace application continuously updates a BCM TX op via TX_SETUP,
bcm_tx_setup() updates the op's timer and count values.

Concurrently, if the timer fires, bcm_tx_timeout_handler() reads them
locklessly in softirq context:

net/can/bcm.c:bcm_tx_timeout_handler() {
    ...
    if (op->kt_ival1 && (op->count > 0)) {
        bcm_can_tx(op, NULL);
        if (!op->count && (op->flags & TX_COUNTEVT)) {
    ...
}

Can this lead to torn reads on 64-bit ktime_t variables (kt_ival1, kt_ival2)
on 32-bit platforms, resulting in a bogus timeout value that breaks cyclic
CAN frame transmission?

Additionally, could a TOCTOU race on op->count cause the handler to
incorrectly miss sending a TX_EXPIRED notification to userspace?

It appears a similar issue for RX_SETUP was previously addressed by adding
locking in commit "can: bcm: add locking when updating filter and timer
values", but the TX_SETUP side might still be exposed.

>  	if (op->flags & TX_ANNOUNCE)
>  		bcm_can_tx(op, NULL);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-bcm_fixes-v3-0-84600ca6d889@hartkopp.net?part=5