ping: annotate data-races in ping_lookup()

isk->inet_num, isk->inet_rcv_saddr and sk->sk_bound_dev_if
are read locklessly in ping_lookup().

Add READ_ONCE()/WRITE_ONCE() annotations.

The race on isk->inet_rcv_saddr is probably coming from IPv6 support,
but does not deserve a specific backport.

Fixes: dbca1596bbb0 ("ping: convert to RCU lookups, get rid of rwlock")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Link: https://patch.msgid.link/20260216100149.3319315-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index ebfc5a3..71d5e17 100644 (file)
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -148,7 +148,7 @@ void ping_unhash(struct sock *sk)
        pr_debug("ping_unhash(isk=%p,isk->num=%u)\n", isk, isk->inet_num);
        spin_lock(&ping_table.lock);
        if (sk_del_node_init_rcu(sk)) {
-               isk->inet_num = 0;
+               WRITE_ONCE(isk->inet_num, 0);
                isk->inet_sport = 0;
                sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1);
        }
@@ -181,31 +181,35 @@ static struct sock *ping_lookup(struct net *net, struct sk_buff *skb, u16 ident)
        }
 
        sk_for_each_rcu(sk, hslot) {
+               int bound_dev_if;
+
                if (!net_eq(sock_net(sk), net))
                        continue;
                isk = inet_sk(sk);
 
                pr_debug("iterate\n");
-               if (isk->inet_num != ident)
+               if (READ_ONCE(isk->inet_num) != ident)
                        continue;
 
+               bound_dev_if = READ_ONCE(sk->sk_bound_dev_if);
                if (skb->protocol == htons(ETH_P_IP) &&
                    sk->sk_family == AF_INET) {
+                       __be32 rcv_saddr = READ_ONCE(isk->inet_rcv_saddr);
+
                        pr_debug("found: %p: num=%d, daddr=%pI4, dif=%d\n", sk,
-                                (int) isk->inet_num, &isk->inet_rcv_saddr,
-                                sk->sk_bound_dev_if);
+                                ident, &rcv_saddr,
+                                bound_dev_if);
 
-                       if (isk->inet_rcv_saddr &&
-                           isk->inet_rcv_saddr != ip_hdr(skb)->daddr)
+                       if (rcv_saddr && rcv_saddr != ip_hdr(skb)->daddr)
                                continue;
 #if IS_ENABLED(CONFIG_IPV6)
                } else if (skb->protocol == htons(ETH_P_IPV6) &&
                           sk->sk_family == AF_INET6) {
 
                        pr_debug("found: %p: num=%d, daddr=%pI6c, dif=%d\n", sk,
-                                (int) isk->inet_num,
+                                ident,
                                 &sk->sk_v6_rcv_saddr,
-                                sk->sk_bound_dev_if);
+                                bound_dev_if);
 
                        if (!ipv6_addr_any(&sk->sk_v6_rcv_saddr) &&
                            !ipv6_addr_equal(&sk->sk_v6_rcv_saddr,
@@ -216,8 +220,8 @@ static struct sock *ping_lookup(struct net *net, struct sk_buff *skb, u16 ident)
                        continue;
                }
 
-               if (sk->sk_bound_dev_if && sk->sk_bound_dev_if != dif &&
-                   sk->sk_bound_dev_if != sdif)
+               if (bound_dev_if && bound_dev_if != dif &&
+                   bound_dev_if != sdif)
                        continue;
 
                goto exit;
@@ -392,7 +396,9 @@ static void ping_set_saddr(struct sock *sk, struct sockaddr_unsized *saddr)
        if (saddr->sa_family == AF_INET) {
                struct inet_sock *isk = inet_sk(sk);
                struct sockaddr_in *addr = (struct sockaddr_in *) saddr;
-               isk->inet_rcv_saddr = isk->inet_saddr = addr->sin_addr.s_addr;
+
+               isk->inet_saddr = addr->sin_addr.s_addr;
+               WRITE_ONCE(isk->inet_rcv_saddr, addr->sin_addr.s_addr);
 #if IS_ENABLED(CONFIG_IPV6)
        } else if (saddr->sa_family == AF_INET6) {
                struct sockaddr_in6 *addr = (struct sockaddr_in6 *) saddr;
@@ -850,7 +856,8 @@ int ping_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int flags,
        struct sk_buff *skb;
        int copied, err;
 
-       pr_debug("ping_recvmsg(sk=%p,sk->num=%u)\n", isk, isk->inet_num);
+       pr_debug("ping_recvmsg(sk=%p,sk->num=%u)\n", isk,
+                READ_ONCE(isk->inet_num));
 
        err = -EOPNOTSUPP;
        if (flags & MSG_OOB)