can: proc: reset pkg_stats atomics individually

Commit 80b5f90158 ("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.

On x86_64 atomic_long_set() is a plain store like memset(), so the
memset() is an actual problem only on architectures with lock-based
atomics (e.g. parisc). The inconsistencies seen on x86_64 come from the
*_delta handling and from the stale snapshots, which are fixed in "can:
proc: skip stats calculation after reset in can_stat_update()". As
before, the counters are reset one at a time and the derived scalar
statistics use plain stores.

Fixes: 80b5f90158 ("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>
Link: https://patch.msgid.link/20261027-can-rx-lock-v12-5-eb5afc2d8b9b@hartkopp.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
This commit is contained in:
Runyu Xiao 2026-10-27 15:31:09 +01:00 committed by Marc Kleine-Budde
parent 43c973a088
commit 12297f855b

View File

@ -76,16 +76,39 @@ 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++;
@ -121,9 +144,10 @@ void can_stat_update(struct timer_list *t)
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)
@ -172,11 +196,6 @@ void can_stat_update(struct timer_list *t)
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));
}