[PATCH v3 3/3] can: statistics: add missing atomic access in hot path

Oliver Hartkopp via B4 Relay posted 3 patches 1 month, 1 week ago
[PATCH v3 3/3] can: statistics: add missing atomic access in hot path
Posted by Oliver Hartkopp via B4 Relay 1 month, 1 week ago
From: Oliver Hartkopp <socketcan@hartkopp.net>

Commit 80b5f90158d1 ("can: statistics: use atomic access in hot path")
fixed a KCSAN issue in can_receive() but missed to convert the 'matches'
variable used in can_rcv_filter().

Fixes: 80b5f90158d1 ("can: statistics: use atomic access in hot path")
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
 net/can/af_can.c | 4 ++--
 net/can/af_can.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/can/af_can.c b/net/can/af_can.c
index 22c65a014861..31a5eab3fab3 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -467,11 +467,11 @@ int can_rx_register(struct net *net, struct net_device *dev, canid_t can_id,
 	dev_rcv_lists = can_dev_rcv_lists_find(net, dev);
 	rcv_list = can_rcv_list_find(&can_id, &mask, dev_rcv_lists);
 
 	rcv->can_id = can_id;
 	rcv->mask = mask;
-	rcv->matches = 0;
+	atomic_long_set(&rcv->matches, 0);
 	rcv->func = func;
 	rcv->data = data;
 	rcv->ident = ident;
 	rcv->sk = sk;
 
@@ -571,11 +571,11 @@ void can_rx_unregister(struct net *net, struct net_device *dev, canid_t can_id,
 EXPORT_SYMBOL(can_rx_unregister);
 
 static inline void deliver(struct sk_buff *skb, struct receiver *rcv)
 {
 	rcv->func(skb, rcv->data);
-	rcv->matches++;
+	atomic_long_inc(&rcv->matches);
 }
 
 static int can_rcv_filter(struct can_dev_rcv_lists *dev_rcv_lists, struct sk_buff *skb)
 {
 	struct receiver *rcv;
diff --git a/net/can/af_can.h b/net/can/af_can.h
index 22f3352c77fe..87887014f562 100644
--- a/net/can/af_can.h
+++ b/net/can/af_can.h
@@ -50,11 +50,11 @@
 
 struct receiver {
 	struct hlist_node list;
 	canid_t can_id;
 	canid_t mask;
-	unsigned long matches;
+	atomic_long_t matches;
 	void (*func)(struct sk_buff *skb, void *data);
 	void *data;
 	char *ident;
 	struct sock *sk;
 	struct rcu_head rcu;

-- 
2.51.0
Re: [PATCH v3 3/3] can: statistics: add missing atomic access in hot path
Posted by Marc Kleine-Budde 1 month ago
The nipa/ai-review bot found these problems:

| https://netdev-ai.bots.linux.dev/ai-review.html?id=873e6dfb-7d8e-4f4d-98ef-a89fcedb701a

commit 083c819fbd59ad774cbf0662b6530ae8cf926328
Author: Oliver Hartkopp <socketcan@hartkopp.net>

can: statistics: add missing atomic access in hot path

This patch converts the matches field in struct receiver from unsigned
long to atomic_long_t to fix a KCSAN race that was missed by commit
80b5f90158d1.

> diff --git a/net/can/af_can.h b/net/can/af_can.h
> --- a/net/can/af_can.h
> +++ b/net/can/af_can.h
> @@ -52,7 +52,7 @@ struct receiver {
>  	struct hlist_node list;
>  	canid_t can_id;
>  	canid_t mask;
> -	unsigned long matches;
> +	atomic_long_t matches;
>  	void (*func)(struct sk_buff *skb, void *data);
>  	void *data;
>  	char *ident;

With this type change, is can_print_rcvlist() in net/can/proc.c also
missing a conversion?  It still accesses r->matches directly:

net/can/proc.c:can_print_rcvlist() {
    ...
    seq_printf(m, fmt, DNAME(dev), r->can_id, r->mask,
               r->func, r->data, r->matches, r->ident);
    ...
}

Should this be atomic_long_read(&r->matches) to match the other
conversion sites?

-- 
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   |