[PATCH can-next v2] can: proc: reset pkg_stats atomics individually

[PATCH can-next v2] can: proc: reset pkg_stats atomics individually

[Oliver Hartkopp]

From: Runyu Xiao <runyu.xiao@seu.edu.cn>

Commit 80b5f90158d1 ("can: statistics: use atomic access in hot path")
converted several members of struct can_pkg_stats to atomic_long_t and
updated the hot TX/RX and procfs read paths to use atomic_long_*()
helpers. However, can_init_stats() still clears the whole struct with
memset().

can_init_stats() is reached from can_stat_update() in timer context and
also from the procfs reset path. Those paths can run while the TX/RX hot
paths are concurrently updating rx_frames, tx_frames, matches and their
*_delta counters. Hitting the whole-struct memset() in that window
performs plain writes to fields that otherwise follow an atomic_long_t
access contract, which can lose or mix live statistics updates.

This issue was found by source-level API-misuse analysis looking for
whole-object resets left behind after atomic_long_t conversions, then
manually audited on Linux v6.18.21.

Replace the whole-struct memset() with a helper that resets the
atomic_long_t counters via atomic_long_set() and clears the derived
scalar statistics explicitly. This preserves the existing reset
semantics for scalar fields while restoring atomic access discipline for
the live counters.

Build-tested by compiling net/can/proc.o on x86_64 netdev/main.
Runtime-tested with a QEMU + vcan setup on Linux v6.18.21 by driving
concurrent traffic and reset_stats reads, which reproduced inconsistent
exported statistics before the fix.

[Hartkopp] can_stat_update() also read the *_delta counters at the
beginning and cleared them with atomic_long_set() at the end. Increments
from the hot path in between were lost for both the current and the next
interval. Use atomic_long_xchg() to fetch and clear the *_delta counters
in one atomic step instead.

Fixes: 80b5f90158d1 ("can: statistics: use atomic access in hot path")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 net/can/proc.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/net/can/proc.c b/net/can/proc.c
index de4d05ae3459..70104f13077e 100644
--- a/net/can/proc.c
+++ b/net/can/proc.c
@@ -74,20 +74,43 @@ static const char rx_list_name[][8] = {
 
 /*
  * af_can statistics stuff
  */
 
+static void can_reset_pkg_stats(struct can_pkg_stats *pkg_stats)
+{
+	atomic_long_set(&pkg_stats->rx_frames, 0);
+	atomic_long_set(&pkg_stats->tx_frames, 0);
+	atomic_long_set(&pkg_stats->matches, 0);
+
+	pkg_stats->total_rx_rate = 0;
+	pkg_stats->total_tx_rate = 0;
+	pkg_stats->total_rx_match_ratio = 0;
+
+	pkg_stats->current_rx_rate = 0;
+	pkg_stats->current_tx_rate = 0;
+	pkg_stats->current_rx_match_ratio = 0;
+
+	pkg_stats->max_rx_rate = 0;
+	pkg_stats->max_tx_rate = 0;
+	pkg_stats->max_rx_match_ratio = 0;
+
+	atomic_long_set(&pkg_stats->rx_frames_delta, 0);
+	atomic_long_set(&pkg_stats->tx_frames_delta, 0);
+	atomic_long_set(&pkg_stats->matches_delta, 0);
+}
+
 static void can_init_stats(struct net *net)
 {
 	struct can_pkg_stats *pkg_stats = net->can.pkg_stats;
 	struct can_rcv_lists_stats *rcv_lists_stats = net->can.rcv_lists_stats;
 	/*
-	 * This memset function is called from a timer context (when
+	 * This stats reset is called from a timer context (when
 	 * can_stattimer is active which is the default) OR in a process
 	 * context (reading the proc_fs when can_stattimer is disabled).
 	 */
-	memset(pkg_stats, 0, sizeof(struct can_pkg_stats));
+	can_reset_pkg_stats(pkg_stats);
 	pkg_stats->jiffies_init = jiffies;
 
 	rcv_lists_stats->stats_reset++;
 
 	if (user_reset) {
@@ -119,13 +142,14 @@ void can_stat_update(struct timer_list *t)
 	unsigned long j = jiffies; /* snapshot */
 
 	long rx_frames = atomic_long_read(&pkg_stats->rx_frames);
 	long tx_frames = atomic_long_read(&pkg_stats->tx_frames);
 	long matches = atomic_long_read(&pkg_stats->matches);
-	long rx_frames_delta = atomic_long_read(&pkg_stats->rx_frames_delta);
-	long tx_frames_delta = atomic_long_read(&pkg_stats->tx_frames_delta);
-	long matches_delta = atomic_long_read(&pkg_stats->matches_delta);
+	/* fetch and clear values for 'current rate' calculation */
+	long rx_frames_delta = atomic_long_xchg(&pkg_stats->rx_frames_delta, 0);
+	long tx_frames_delta = atomic_long_xchg(&pkg_stats->tx_frames_delta, 0);
+	long matches_delta = atomic_long_xchg(&pkg_stats->matches_delta, 0);
 
 	/* restart counting in timer context on user request */
 	if (user_reset)
 		can_init_stats(net);
 
@@ -170,15 +194,10 @@ void can_stat_update(struct timer_list *t)
 		pkg_stats->max_rx_rate = pkg_stats->current_rx_rate;
 
 	if (pkg_stats->max_rx_match_ratio < pkg_stats->current_rx_match_ratio)
 		pkg_stats->max_rx_match_ratio = pkg_stats->current_rx_match_ratio;
 
-	/* clear values for 'current rate' calculation */
-	atomic_long_set(&pkg_stats->tx_frames_delta, 0);
-	atomic_long_set(&pkg_stats->rx_frames_delta, 0);
-	atomic_long_set(&pkg_stats->matches_delta, 0);
-
 	/* restart timer (one second) */
 	mod_timer(&net->can.stattimer, round_jiffies(jiffies + HZ));
 }
 
 /*
-- 
2.53.0

Re: [PATCH can-next v2] can: proc: reset pkg_stats atomics individually

[Marc Kleine-Budde]

On Fri, 16 Oct 2026 13:09:05 +0200, Oliver Hartkopp wrote:
> can: proc: reset pkg_stats atomics individually

Applied, thanks!

[1/1] can: proc: reset pkg_stats atomics individually
      commit: b2e10ad6fda77450cb6c49dcd4032140395a2b16

Best regards,
-- 
Marc Kleine-Budde <mkl@pengutronix.de>

Re: [PATCH can-next v2] can: proc: reset pkg_stats atomics individually

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 3001 bytes --]

On 16.10.2026 13:09:05, Oliver Hartkopp wrote:
> From: Runyu Xiao <runyu.xiao@seu.edu.cn>
>
> Commit 80b5f90158d1 ("can: statistics: use atomic access in hot path")
> converted several members of struct can_pkg_stats to atomic_long_t and
> updated the hot TX/RX and procfs read paths to use atomic_long_*()
> helpers. However, can_init_stats() still clears the whole struct with
> memset().
>
> can_init_stats() is reached from can_stat_update() in timer context and
> also from the procfs reset path. Those paths can run while the TX/RX hot
> paths are concurrently updating rx_frames, tx_frames, matches and their
> *_delta counters. Hitting the whole-struct memset() in that window
> performs plain writes to fields that otherwise follow an atomic_long_t
> access contract, which can lose or mix live statistics updates.
>
> This issue was found by source-level API-misuse analysis looking for
> whole-object resets left behind after atomic_long_t conversions, then
> manually audited on Linux v6.18.21.
>
> Replace the whole-struct memset() with a helper that resets the
> atomic_long_t counters via atomic_long_set() and clears the derived
> scalar statistics explicitly. This preserves the existing reset
> semantics for scalar fields while restoring atomic access discipline for
> the live counters.
>
> Build-tested by compiling net/can/proc.o on x86_64 netdev/main.
> Runtime-tested with a QEMU + vcan setup on Linux v6.18.21 by driving
> concurrent traffic and reset_stats reads, which reproduced inconsistent
> exported statistics before the fix.
>
> [Hartkopp] can_stat_update() also read the *_delta counters at the
  ^^^^^^^^^^

As this is a regular vN+1 patch posted by you to the mailing list,
please don't add your [tag] here.

If you want, you can add a change log entry after the "---"

Marc

> beginning and cleared them with atomic_long_set() at the end. Increments
> from the hot path in between were lost for both the current and the next
> interval. Use atomic_long_xchg() to fetch and clear the *_delta counters
> in one atomic step instead.
>
> Fixes: 80b5f90158d1 ("can: statistics: use atomic access in hot path")
> Cc: stable@vger.kernel.org
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---

Change log goes here.

FYI: you can use b4 to pick up patches from the ML:

| b4 prep -n proc-pkg_stats-atomics -F 20260512133937.21957-1-runyu.xiao@seu.edu.cn -f net-next/main

Add the change log to the cover letter as usual, b4 will merge cover
letter and patch for a "series" consisting of a single patch only.

regards,
Marc

--
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH can-next v2] can: proc: reset pkg_stats atomics individually

[Oliver Hartkopp]

On 16.10.26 13:36, Marc Kleine-Budde wrote:
> On 16.10.2026 13:09:05, Oliver Hartkopp wrote:
>> From: Runyu Xiao <runyu.xiao@seu.edu.cn>
>>
>> Commit 80b5f90158d1 ("can: statistics: use atomic access in hot path")
>> converted several members of struct can_pkg_stats to atomic_long_t and
>> updated the hot TX/RX and procfs read paths to use atomic_long_*()
>> helpers. However, can_init_stats() still clears the whole struct with
>> memset().
>>
>> can_init_stats() is reached from can_stat_update() in timer context and
>> also from the procfs reset path. Those paths can run while the TX/RX hot
>> paths are concurrently updating rx_frames, tx_frames, matches and their
>> *_delta counters. Hitting the whole-struct memset() in that window
>> performs plain writes to fields that otherwise follow an atomic_long_t
>> access contract, which can lose or mix live statistics updates.
>>
>> This issue was found by source-level API-misuse analysis looking for
>> whole-object resets left behind after atomic_long_t conversions, then
>> manually audited on Linux v6.18.21.
>>
>> Replace the whole-struct memset() with a helper that resets the
>> atomic_long_t counters via atomic_long_set() and clears the derived
>> scalar statistics explicitly. This preserves the existing reset
>> semantics for scalar fields while restoring atomic access discipline for
>> the live counters.
>>
>> Build-tested by compiling net/can/proc.o on x86_64 netdev/main.
>> Runtime-tested with a QEMU + vcan setup on Linux v6.18.21 by driving
>> concurrent traffic and reset_stats reads, which reproduced inconsistent
>> exported statistics before the fix.
>>
>> [Hartkopp] can_stat_update() also read the *_delta counters at the
>    ^^^^^^^^^^
> 
> As this is a regular vN+1 patch posted by you to the mailing list,
> please don't add your [tag] here.
> 
> If you want, you can add a change log entry after the "---"

I would have done so, if I was the original author.

But in this case I documented my personal contribution and also my S-o-b.

Otherwise this would not have been transparent to the reader.

Best regards,
Oliver

> 
> Marc
> 
>> beginning and cleared them with atomic_long_set() at the end. Increments
>> from the hot path in between were lost for both the current and the next
>> interval. Use atomic_long_xchg() to fetch and clear the *_delta counters
>> in one atomic step instead.
>>
>> Fixes: 80b5f90158d1 ("can: statistics: use atomic access in hot path")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
>> Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>> ---
> 
> Change log goes here.
> 
> FYI: you can use b4 to pick up patches from the ML:
> 
> | b4 prep -n proc-pkg_stats-atomics -F 20260512133937.21957-1-runyu.xiao@seu.edu.cn -f net-next/main
> 
> Add the change log to the cover letter as usual, b4 will merge cover
> letter and patch for a "series" consisting of a single patch only.
> 
> regards,
> Marc
> 
> --
> Pengutronix e.K.                 | Marc Kleine-Budde          |
> Embedded Linux                   | https://www.pengutronix.de |
> Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |