Skip to content

Commit 31be0d5

Browse files
edumazetSasha Levin
authored andcommitted
ping: annotate data-races in ping_lookup()
[ Upstream commit ad5dfde2a5733aaf652ea3e40c8c5e071e935901 ] 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: dbca159 ("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> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 469b7ae commit 31be0d5

1 file changed

Lines changed: 19 additions & 12 deletions

File tree

net/ipv4/ping.c

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ void ping_unhash(struct sock *sk)
159159
pr_debug("ping_unhash(isk=%p,isk->num=%u)\n", isk, isk->inet_num);
160160
spin_lock(&ping_table.lock);
161161
if (sk_del_node_init_rcu(sk)) {
162-
isk->inet_num = 0;
162+
WRITE_ONCE(isk->inet_num, 0);
163163
isk->inet_sport = 0;
164164
sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1);
165165
}
@@ -192,31 +192,35 @@ static struct sock *ping_lookup(struct net *net, struct sk_buff *skb, u16 ident)
192192
}
193193

194194
sk_for_each_rcu(sk, hslot) {
195+
int bound_dev_if;
196+
195197
if (!net_eq(sock_net(sk), net))
196198
continue;
197199
isk = inet_sk(sk);
198200

199201
pr_debug("iterate\n");
200-
if (isk->inet_num != ident)
202+
if (READ_ONCE(isk->inet_num) != ident)
201203
continue;
202204

205+
bound_dev_if = READ_ONCE(sk->sk_bound_dev_if);
203206
if (skb->protocol == htons(ETH_P_IP) &&
204207
sk->sk_family == AF_INET) {
208+
__be32 rcv_saddr = READ_ONCE(isk->inet_rcv_saddr);
209+
205210
pr_debug("found: %p: num=%d, daddr=%pI4, dif=%d\n", sk,
206-
(int) isk->inet_num, &isk->inet_rcv_saddr,
207-
sk->sk_bound_dev_if);
211+
ident, &rcv_saddr,
212+
bound_dev_if);
208213

209-
if (isk->inet_rcv_saddr &&
210-
isk->inet_rcv_saddr != ip_hdr(skb)->daddr)
214+
if (rcv_saddr && rcv_saddr != ip_hdr(skb)->daddr)
211215
continue;
212216
#if IS_ENABLED(CONFIG_IPV6)
213217
} else if (skb->protocol == htons(ETH_P_IPV6) &&
214218
sk->sk_family == AF_INET6) {
215219

216220
pr_debug("found: %p: num=%d, daddr=%pI6c, dif=%d\n", sk,
217-
(int) isk->inet_num,
221+
ident,
218222
&sk->sk_v6_rcv_saddr,
219-
sk->sk_bound_dev_if);
223+
bound_dev_if);
220224

221225
if (!ipv6_addr_any(&sk->sk_v6_rcv_saddr) &&
222226
!ipv6_addr_equal(&sk->sk_v6_rcv_saddr,
@@ -227,8 +231,8 @@ static struct sock *ping_lookup(struct net *net, struct sk_buff *skb, u16 ident)
227231
continue;
228232
}
229233

230-
if (sk->sk_bound_dev_if && sk->sk_bound_dev_if != dif &&
231-
sk->sk_bound_dev_if != sdif)
234+
if (bound_dev_if && bound_dev_if != dif &&
235+
bound_dev_if != sdif)
232236
continue;
233237

234238
goto exit;
@@ -403,7 +407,9 @@ static void ping_set_saddr(struct sock *sk, struct sockaddr *saddr)
403407
if (saddr->sa_family == AF_INET) {
404408
struct inet_sock *isk = inet_sk(sk);
405409
struct sockaddr_in *addr = (struct sockaddr_in *) saddr;
406-
isk->inet_rcv_saddr = isk->inet_saddr = addr->sin_addr.s_addr;
410+
411+
isk->inet_saddr = addr->sin_addr.s_addr;
412+
WRITE_ONCE(isk->inet_rcv_saddr, addr->sin_addr.s_addr);
407413
#if IS_ENABLED(CONFIG_IPV6)
408414
} else if (saddr->sa_family == AF_INET6) {
409415
struct sockaddr_in6 *addr = (struct sockaddr_in6 *) saddr;
@@ -865,7 +871,8 @@ int ping_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int flags,
865871
struct sk_buff *skb;
866872
int copied, err;
867873

868-
pr_debug("ping_recvmsg(sk=%p,sk->num=%u)\n", isk, isk->inet_num);
874+
pr_debug("ping_recvmsg(sk=%p,sk->num=%u)\n", isk,
875+
READ_ONCE(isk->inet_num));
869876

870877
err = -EOPNOTSUPP;
871878
if (flags & MSG_OOB)

0 commit comments

Comments
 (0)