can: statistics: use atomic access in hot path

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>

diff --git a/net/can/af_can.c b/net/can/af_can.c
index 01f3fbb3b67dc687a82661395c5f99e325c54120..65230e81fa08c95f945397e3c7fd9d70b0ab7d70 100644 (file)
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -287,8 +287,8 @@ int can_send(struct sk_buff *skb, int loop)
                netif_rx(newskb);
 
        /* update statistics */
-       pkg_stats->tx_frames++;
-       pkg_stats->tx_frames_delta++;
+       atomic_long_inc(&pkg_stats->tx_frames);
+       atomic_long_inc(&pkg_stats->tx_frames_delta);
 
        return 0;
 
@@ -647,8 +647,8 @@ static void can_receive(struct sk_buff *skb, struct net_device *dev)
        int matches;
 
        /* update statistics */
-       pkg_stats->rx_frames++;
-       pkg_stats->rx_frames_delta++;
+       atomic_long_inc(&pkg_stats->rx_frames);
+       atomic_long_inc(&pkg_stats->rx_frames_delta);
 
        /* create non-zero unique skb identifier together with *skb */
        while (!(can_skb_prv(skb)->skbcnt))
@@ -669,8 +669,8 @@ static void can_receive(struct sk_buff *skb, struct net_device *dev)
        consume_skb(skb);
 
        if (matches > 0) {
-               pkg_stats->matches++;
-               pkg_stats->matches_delta++;
+               atomic_long_inc(&pkg_stats->matches);
+               atomic_long_inc(&pkg_stats->matches_delta);
        }
 }
 

diff --git a/net/can/af_can.h b/net/can/af_can.h
index 7c2d9161e22457076d0802cd79877bcd7bdfa35d..22f3352c77fecec1bb973715dabe83021db3a2f2 100644 (file)
--- a/net/can/af_can.h
+++ b/net/can/af_can.h
@@ -66,9 +66,9 @@ struct receiver {
 struct can_pkg_stats {
        unsigned long jiffies_init;
 
-       unsigned long rx_frames;
-       unsigned long tx_frames;
-       unsigned long matches;
+       atomic_long_t rx_frames;
+       atomic_long_t tx_frames;
+       atomic_long_t matches;
 
        unsigned long total_rx_rate;
        unsigned long total_tx_rate;
@@ -82,9 +82,9 @@ struct can_pkg_stats {
        unsigned long max_tx_rate;
        unsigned long max_rx_match_ratio;
 
-       unsigned long rx_frames_delta;
-       unsigned long tx_frames_delta;
-       unsigned long matches_delta;
+       atomic_long_t rx_frames_delta;
+       atomic_long_t tx_frames_delta;
+       atomic_long_t matches_delta;
 };
 
 /* persistent statistics */

diff --git a/net/can/proc.c b/net/can/proc.c
index bbce97825f13fbf725261e572271a9353d51897d..25fdf060e30d0d5133fc1d5e0c5cfd5c46233bb0 100644 (file)
--- a/net/can/proc.c
+++ b/net/can/proc.c
@@ -118,6 +118,13 @@ void can_stat_update(struct timer_list *t)
        struct can_pkg_stats *pkg_stats = net->can.pkg_stats;
        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 */
        if (user_reset)
                can_init_stats(net);
@@ -127,35 +134,33 @@ void can_stat_update(struct timer_list *t)
                can_init_stats(net);
 
        /* prevent overflow in calc_rate() */
-       if (pkg_stats->rx_frames > (ULONG_MAX / HZ))
+       if (rx_frames > (LONG_MAX / HZ))
                can_init_stats(net);
 
        /* prevent overflow in calc_rate() */
-       if (pkg_stats->tx_frames > (ULONG_MAX / HZ))
+       if (tx_frames > (LONG_MAX / HZ))
                can_init_stats(net);
 
        /* matches overflow - very improbable */
-       if (pkg_stats->matches > (ULONG_MAX / 100))
+       if (matches > (LONG_MAX / 100))
                can_init_stats(net);
 
        /* calc total values */
-       if (pkg_stats->rx_frames)
-               pkg_stats->total_rx_match_ratio = (pkg_stats->matches * 100) /
-                       pkg_stats->rx_frames;
+       if (rx_frames)
+               pkg_stats->total_rx_match_ratio = (matches * 100) / rx_frames;
 
        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->rx_frames);
+                                           rx_frames);
 
        /* calc current values */
-       if (pkg_stats->rx_frames_delta)
+       if (rx_frames_delta)
                pkg_stats->current_rx_match_ratio =
-                       (pkg_stats->matches_delta * 100) /
-                       pkg_stats->rx_frames_delta;
+                       (matches_delta * 100) / rx_frames_delta;
 
-       pkg_stats->current_tx_rate = calc_rate(0, HZ, pkg_stats->tx_frames_delta);
-       pkg_stats->current_rx_rate = calc_rate(0, HZ, pkg_stats->rx_frames_delta);
+       pkg_stats->current_tx_rate = calc_rate(0, HZ, tx_frames_delta);
+       pkg_stats->current_rx_rate = calc_rate(0, HZ, rx_frames_delta);
 
        /* check / update maximum values */
        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;
 
        /* clear values for 'current rate' calculation */
-       pkg_stats->tx_frames_delta = 0;
-       pkg_stats->rx_frames_delta = 0;
-       pkg_stats->matches_delta   = 0;
+       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));
@@ -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;
 
        seq_putc(m, '\n');
-       seq_printf(m, " %8ld transmitted frames (TXF)\n", pkg_stats->tx_frames);
-       seq_printf(m, " %8ld received frames (RXF)\n", pkg_stats->rx_frames);
-       seq_printf(m, " %8ld matched frames (RXMF)\n", pkg_stats->matches);
+       seq_printf(m, " %8ld transmitted frames (TXF)\n",
+                  atomic_long_read(&pkg_stats->tx_frames));
+       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');