can: statistics: use atomic access in hot path

[ Upstream commit 80b5f90158d1364cbd80ad82852a757fc0692bf2 ]

In can_send() and can_receive() CAN messages and CAN filter matches are
counted to be visible in the CAN procfs files.

KCSAN detected a data race within can_send() when two CAN frames have
been generated by a timer event writing to the same CAN netdevice at the
same time. Use atomic operations to access the statistics in the hot path
to fix the KCSAN complaint.

Reported-by: syzbot+78ce4489b812515d5e4d@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/67cd717d.050a0220.e1a89.0006.GAE@google.com
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://patch.msgid.link/20250310143353.3242-1-socketcan@hartkopp.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
Oliver Hartkopp 2025-03-10 15:33:53 +01:00 committed by Greg Kroah-Hartman
parent 1a8bae6723
commit 4b07ce83e9
3 changed files with 39 additions and 31 deletions

View File

@ -287,8 +287,8 @@ int can_send(struct sk_buff *skb, int loop)
netif_rx(newskb); netif_rx(newskb);
/* update statistics */ /* update statistics */
pkg_stats->tx_frames++; atomic_long_inc(&pkg_stats->tx_frames);
pkg_stats->tx_frames_delta++; atomic_long_inc(&pkg_stats->tx_frames_delta);
return 0; return 0;
@ -648,8 +648,8 @@ static void can_receive(struct sk_buff *skb, struct net_device *dev)
int matches; int matches;
/* update statistics */ /* update statistics */
pkg_stats->rx_frames++; atomic_long_inc(&pkg_stats->rx_frames);
pkg_stats->rx_frames_delta++; atomic_long_inc(&pkg_stats->rx_frames_delta);
/* create non-zero unique skb identifier together with *skb */ /* create non-zero unique skb identifier together with *skb */
while (!(can_skb_prv(skb)->skbcnt)) while (!(can_skb_prv(skb)->skbcnt))
@ -670,8 +670,8 @@ static void can_receive(struct sk_buff *skb, struct net_device *dev)
consume_skb(skb); consume_skb(skb);
if (matches > 0) { if (matches > 0) {
pkg_stats->matches++; atomic_long_inc(&pkg_stats->matches);
pkg_stats->matches_delta++; atomic_long_inc(&pkg_stats->matches_delta);
} }
} }

View File

@ -66,9 +66,9 @@ struct receiver {
struct can_pkg_stats { struct can_pkg_stats {
unsigned long jiffies_init; unsigned long jiffies_init;
unsigned long rx_frames; atomic_long_t rx_frames;
unsigned long tx_frames; atomic_long_t tx_frames;
unsigned long matches; atomic_long_t matches;
unsigned long total_rx_rate; unsigned long total_rx_rate;
unsigned long total_tx_rate; unsigned long total_tx_rate;
@ -82,9 +82,9 @@ struct can_pkg_stats {
unsigned long max_tx_rate; unsigned long max_tx_rate;
unsigned long max_rx_match_ratio; unsigned long max_rx_match_ratio;
unsigned long rx_frames_delta; atomic_long_t rx_frames_delta;
unsigned long tx_frames_delta; atomic_long_t tx_frames_delta;
unsigned long matches_delta; atomic_long_t matches_delta;
}; };
/* persistent statistics */ /* persistent statistics */

View File

@ -118,6 +118,13 @@ void can_stat_update(struct timer_list *t)
struct can_pkg_stats *pkg_stats = net->can.pkg_stats; struct can_pkg_stats *pkg_stats = net->can.pkg_stats;
unsigned long j = jiffies; /* snapshot */ 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);
/* restart counting in timer context on user request */ /* restart counting in timer context on user request */
if (user_reset) if (user_reset)
can_init_stats(net); can_init_stats(net);
@ -127,35 +134,33 @@ void can_stat_update(struct timer_list *t)
can_init_stats(net); can_init_stats(net);
/* prevent overflow in calc_rate() */ /* prevent overflow in calc_rate() */
if (pkg_stats->rx_frames > (ULONG_MAX / HZ)) if (rx_frames > (LONG_MAX / HZ))
can_init_stats(net); can_init_stats(net);
/* prevent overflow in calc_rate() */ /* prevent overflow in calc_rate() */
if (pkg_stats->tx_frames > (ULONG_MAX / HZ)) if (tx_frames > (LONG_MAX / HZ))
can_init_stats(net); can_init_stats(net);
/* matches overflow - very improbable */ /* matches overflow - very improbable */
if (pkg_stats->matches > (ULONG_MAX / 100)) if (matches > (LONG_MAX / 100))
can_init_stats(net); can_init_stats(net);
/* calc total values */ /* calc total values */
if (pkg_stats->rx_frames) if (rx_frames)
pkg_stats->total_rx_match_ratio = (pkg_stats->matches * 100) / pkg_stats->total_rx_match_ratio = (matches * 100) / rx_frames;
pkg_stats->rx_frames;
pkg_stats->total_tx_rate = calc_rate(pkg_stats->jiffies_init, j, pkg_stats->total_tx_rate = calc_rate(pkg_stats->jiffies_init, j,
pkg_stats->tx_frames); tx_frames);
pkg_stats->total_rx_rate = calc_rate(pkg_stats->jiffies_init, j, pkg_stats->total_rx_rate = calc_rate(pkg_stats->jiffies_init, j,
pkg_stats->rx_frames); rx_frames);
/* calc current values */ /* calc current values */
if (pkg_stats->rx_frames_delta) if (rx_frames_delta)
pkg_stats->current_rx_match_ratio = pkg_stats->current_rx_match_ratio =
(pkg_stats->matches_delta * 100) / (matches_delta * 100) / rx_frames_delta;
pkg_stats->rx_frames_delta;
pkg_stats->current_tx_rate = calc_rate(0, HZ, pkg_stats->tx_frames_delta); pkg_stats->current_tx_rate = calc_rate(0, HZ, tx_frames_delta);
pkg_stats->current_rx_rate = calc_rate(0, HZ, pkg_stats->rx_frames_delta); pkg_stats->current_rx_rate = calc_rate(0, HZ, rx_frames_delta);
/* check / update maximum values */ /* check / update maximum values */
if (pkg_stats->max_tx_rate < pkg_stats->current_tx_rate) if (pkg_stats->max_tx_rate < pkg_stats->current_tx_rate)
@ -168,9 +173,9 @@ void can_stat_update(struct timer_list *t)
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 */ /* clear values for 'current rate' calculation */
pkg_stats->tx_frames_delta = 0; atomic_long_set(&pkg_stats->tx_frames_delta, 0);
pkg_stats->rx_frames_delta = 0; atomic_long_set(&pkg_stats->rx_frames_delta, 0);
pkg_stats->matches_delta = 0; atomic_long_set(&pkg_stats->matches_delta, 0);
/* restart timer (one second) */ /* restart timer (one second) */
mod_timer(&net->can.stattimer, round_jiffies(jiffies + HZ)); mod_timer(&net->can.stattimer, round_jiffies(jiffies + HZ));
@ -214,9 +219,12 @@ static int can_stats_proc_show(struct seq_file *m, void *v)
struct can_rcv_lists_stats *rcv_lists_stats = net->can.rcv_lists_stats; struct can_rcv_lists_stats *rcv_lists_stats = net->can.rcv_lists_stats;
seq_putc(m, '\n'); seq_putc(m, '\n');
seq_printf(m, " %8ld transmitted frames (TXF)\n", pkg_stats->tx_frames); seq_printf(m, " %8ld transmitted frames (TXF)\n",
seq_printf(m, " %8ld received frames (RXF)\n", pkg_stats->rx_frames); atomic_long_read(&pkg_stats->tx_frames));
seq_printf(m, " %8ld matched frames (RXMF)\n", pkg_stats->matches); seq_printf(m, " %8ld received frames (RXF)\n",
atomic_long_read(&pkg_stats->rx_frames));
seq_printf(m, " %8ld matched frames (RXMF)\n",
atomic_long_read(&pkg_stats->matches));
seq_putc(m, '\n'); seq_putc(m, '\n');