1. Patchew
  2. linux
  3. View series

[PATCH net v2] raw: annotate data-races in raw_v4_match()

Runyu Xiao posted 1 patch 5 days, 10 hours ago 
net/ipv4/raw.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
[PATCH net v2] raw: annotate data-races in raw_v4_match()
Posted by Runyu Xiao 5 days, 10 hours ago 
raw_v4_match() is a lockless match helper under sk_for_each_rcu()
and still reads inet->inet_num, inet->inet_rcv_saddr and
sk->sk_bound_dev_if with plain loads.

inet_num and sk_bound_dev_if already have WRITE_ONCE() writers, and
raw_bind() should use WRITE_ONCE() for inet_rcv_saddr as well.

Add matching READ_ONCE()/WRITE_ONCE() annotations.

Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
v2:
- Note that inet_num and sk_bound_dev_if already have WRITE_ONCE() writers
- Add WRITE_ONCE() in raw_bind() for inet_rcv_saddr
- Previous version: https://lore.kernel.org/r/20260601073937.1137673-1-runyu.xiao@seu.edu.cn

 net/ipv4/raw.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 5aaf9c62c8e1..faa49f3d239a 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -120,11 +120,14 @@ bool raw_v4_match(struct net *net, const struct sock *sk, unsigned short num,
 		  __be32 raddr, __be32 laddr, int dif, int sdif)
 {
 	const struct inet_sock *inet = inet_sk(sk);
+	unsigned short match_num = READ_ONCE(inet->inet_num);
+	__be32 match_rcv_saddr = READ_ONCE(inet->inet_rcv_saddr);
+	int match_bound_dev_if = READ_ONCE(sk->sk_bound_dev_if);
 
-	if (net_eq(sock_net(sk), net) && inet->inet_num == num	&&
+	if (net_eq(sock_net(sk), net) && match_num == num &&
 	    !(inet->inet_daddr && inet->inet_daddr != raddr) 	&&
-	    !(inet->inet_rcv_saddr && inet->inet_rcv_saddr != laddr) &&
-	    raw_sk_bound_dev_eq(net, sk->sk_bound_dev_if, dif, sdif))
+	    !(match_rcv_saddr && match_rcv_saddr != laddr) &&
+	    raw_sk_bound_dev_eq(net, match_bound_dev_if, dif, sdif))
 		return true;
 	return false;
 }
@@ -727,7 +730,8 @@ static int raw_bind(struct sock *sk, struct sockaddr_unsized *uaddr,
 	if (!inet_addr_valid_or_nonlocal(net, inet, addr->sin_addr.s_addr,
 					 chk_addr_ret))
 		goto out;
 
-	inet->inet_rcv_saddr = inet->inet_saddr = addr->sin_addr.s_addr;
+	inet->inet_saddr = addr->sin_addr.s_addr;
+	WRITE_ONCE(inet->inet_rcv_saddr, addr->sin_addr.s_addr);
 	if (chk_addr_ret == RTN_MULTICAST || chk_addr_ret == RTN_BROADCAST)
 		inet->inet_saddr = 0;  /* Use device */
 	sk_dst_reset(sk);
-- 
2.34.1
Re: [PATCH net v2] raw: annotate data-races in raw_v4_match()
Posted by Eric Dumazet 5 days, 10 hours ago 
On Tue, Jun 2, 2026 at 9:27 AM Runyu Xiao <runyu.xiao@seu.edu.cn> wrote:
>
> raw_v4_match() is a lockless match helper under sk_for_each_rcu()
> and still reads inet->inet_num, inet->inet_rcv_saddr and
> sk->sk_bound_dev_if with plain loads.
>
> inet_num and sk_bound_dev_if already have WRITE_ONCE() writers, and
> raw_bind() should use WRITE_ONCE() for inet_rcv_saddr as well.
>
> Add matching READ_ONCE()/WRITE_ONCE() annotations.
>
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
> v2:
> - Note that inet_num and sk_bound_dev_if already have WRITE_ONCE() writers
> - Add WRITE_ONCE() in raw_bind() for inet_rcv_saddr
> - Previous version: https://lore.kernel.org/r/20260601073937.1137673-1-runyu.xiao@seu.edu.cn
>
>  net/ipv4/raw.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
> index 5aaf9c62c8e1..faa49f3d239a 100644
> --- a/net/ipv4/raw.c
> +++ b/net/ipv4/raw.c
> @@ -120,11 +120,14 @@ bool raw_v4_match(struct net *net, const struct sock *sk, unsigned short num,
>                   __be32 raddr, __be32 laddr, int dif, int sdif)
>  {
>         const struct inet_sock *inet = inet_sk(sk);
> +       unsigned short match_num = READ_ONCE(inet->inet_num);

I think inet->inet_num is immutable, raw sockets have SOCK_RCU_FREE.

> +       __be32 match_rcv_saddr = READ_ONCE(inet->inet_rcv_saddr);
> +       int match_bound_dev_if = READ_ONCE(sk->sk_bound_dev_if);

Why do you need a local variable to store sk->sk_bound_dev_if ?

This forces the compiler to read it (in a register) or store it in a
temporary variable
before tests that might short-circuit the read (eg if network
namespace does not match).

>
> -       if (net_eq(sock_net(sk), net) && inet->inet_num == num  &&
> +       if (net_eq(sock_net(sk), net) && match_num == num &&
>             !(inet->inet_daddr && inet->inet_daddr != raddr)    &&

Why isn't inet_daddr covered in your patch?

> -           !(inet->inet_rcv_saddr && inet->inet_rcv_saddr != laddr) &&
> -           raw_sk_bound_dev_eq(net, sk->sk_bound_dev_if, dif, sdif))
> +           !(match_rcv_saddr && match_rcv_saddr != laddr) &&
> +           raw_sk_bound_dev_eq(net, match_bound_dev_if, dif, sdif))
>                 return true;
>         return false;
>  }
> @@ -727,7 +730,8 @@ static int raw_bind(struct sock *sk, struct sockaddr_unsized *uaddr,
>         if (!inet_addr_valid_or_nonlocal(net, inet, addr->sin_addr.s_addr,
>                                          chk_addr_ret))
>                 goto out;
>
> -       inet->inet_rcv_saddr = inet->inet_saddr = addr->sin_addr.s_addr;
> +       inet->inet_saddr = addr->sin_addr.s_addr;
> +       WRITE_ONCE(inet->inet_rcv_saddr, addr->sin_addr.s_addr);
>         if (chk_addr_ret == RTN_MULTICAST || chk_addr_ret == RTN_BROADCAST)
>                 inet->inet_saddr = 0;  /* Use device */
>         sk_dst_reset(sk);
> --
> 2.34.1

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.