Commit 80b5f901 authored by Oliver Hartkopp's avatar Oliver Hartkopp Committed by Marc Kleine-Budde
Browse files

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: default avatar <syzbot+78ce4489b812515d5e4d@syzkaller.appspotmail.com>
Closes: https://lore.kernel.org/all/67cd717d.050a0220.e1a89.0006.GAE@google.com


Signed-off-by: default avatarOliver Hartkopp <socketcan@hartkopp.net>
Reviewed-by: default avatarVincent Mailhol <mailhol.vincent@wanadoo.fr>
Link: https://patch.msgid.link/20250310143353.3242-1-socketcan@hartkopp.net


Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parent 1d22a122

net/can/af_can.c

+6 −6
Original line number Diff line number Diff line
@@ -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);

	}

}

net/can/af_can.h

+6 −6
Original line number Diff line number Diff line
@@ -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 */

net/can/proc.c

+27 −19
Original line number Diff line number Diff line
@@ -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');