lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20231208101244.1019034-3-edumazet@google.com>
Date: Fri,  8 Dec 2023 10:12:44 +0000
From: Eric Dumazet <edumazet@...gle.com>
To: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>
Cc: David Ahern <dsahern@...nel.org>, netdev@...r.kernel.org, eric.dumazet@...il.com, 
	Eric Dumazet <edumazet@...gle.com>
Subject: [PATCH net-next 2/2] ipv6: annotate data-races around np->ucast_oif

np->ucast_oif is read locklessly in some contexts.

Make all accesses to this field lockless, adding appropriate
annotations.

This also makes setsockopt( IPV6_UNICAST_IF ) lockless.

Signed-off-by: Eric Dumazet <edumazet@...gle.com>
---
 net/ipv6/datagram.c      |  2 +-
 net/ipv6/icmp.c          |  4 +--
 net/ipv6/ipv6_sockglue.c | 58 ++++++++++++++++++----------------------
 net/ipv6/ping.c          |  4 +--
 net/ipv6/raw.c           |  2 +-
 net/ipv6/udp.c           |  2 +-
 net/l2tp/l2tp_ip6.c      |  2 +-
 7 files changed, 34 insertions(+), 40 deletions(-)

diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 1804bd6f46840f39deb3ceeb7835cd167e1ec86c..fff78496803da6158d8b6e70255a56f183e26a80 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -62,7 +62,7 @@ static void ip6_datagram_flow_key_init(struct flowi6 *fl6,
 		if (ipv6_addr_is_multicast(&fl6->daddr))
 			oif = READ_ONCE(np->mcast_oif);
 		else
-			oif = np->ucast_oif;
+			oif = READ_ONCE(np->ucast_oif);
 	}
 
 	fl6->flowi6_oif = oif;
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index f84a465c9759b6c3d43a80a65dac32d516219c60..1635da07285f263509a68624369a2746f3deb076 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -586,7 +586,7 @@ void icmp6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info,
 	if (!fl6.flowi6_oif && ipv6_addr_is_multicast(&fl6.daddr))
 		fl6.flowi6_oif = READ_ONCE(np->mcast_oif);
 	else if (!fl6.flowi6_oif)
-		fl6.flowi6_oif = np->ucast_oif;
+		fl6.flowi6_oif = READ_ONCE(np->ucast_oif);
 
 	ipcm6_init_sk(&ipc6, sk);
 	ipc6.sockc.mark = mark;
@@ -772,7 +772,7 @@ static enum skb_drop_reason icmpv6_echo_reply(struct sk_buff *skb)
 	if (!fl6.flowi6_oif && ipv6_addr_is_multicast(&fl6.daddr))
 		fl6.flowi6_oif = READ_ONCE(np->mcast_oif);
 	else if (!fl6.flowi6_oif)
-		fl6.flowi6_oif = np->ucast_oif;
+		fl6.flowi6_oif = READ_ONCE(np->ucast_oif);
 
 	if (ip6_dst_lookup(net, sk, &dst, &fl6))
 		goto out;
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index fe7e96e69960c013e84b48242e309525f7f618da..9e8ebda170f14f7fd5faf370507bb3a8d1c75931 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -537,6 +537,31 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
 		}
 		WRITE_ONCE(np->mcast_oif, val);
 		return 0;
+	case IPV6_UNICAST_IF:
+	{
+		struct net_device *dev;
+		int ifindex;
+
+		if (optlen != sizeof(int))
+			return -EINVAL;
+
+		ifindex = (__force int)ntohl((__force __be32)val);
+		if (!ifindex) {
+			WRITE_ONCE(np->ucast_oif, 0);
+			return 0;
+		}
+
+		dev = dev_get_by_index(net, ifindex);
+		if (!dev)
+			return -EADDRNOTAVAIL;
+		dev_put(dev);
+
+		if (READ_ONCE(sk->sk_bound_dev_if))
+			return -EINVAL;
+
+		WRITE_ONCE(np->ucast_oif, ifindex);
+		return 0;
+	}
 	}
 	if (needs_rtnl)
 		rtnl_lock();
@@ -857,37 +882,6 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
 		break;
 	}
 
-
-	case IPV6_UNICAST_IF:
-	{
-		struct net_device *dev = NULL;
-		int ifindex;
-
-		if (optlen != sizeof(int))
-			goto e_inval;
-
-		ifindex = (__force int)ntohl((__force __be32)val);
-		if (ifindex == 0) {
-			np->ucast_oif = 0;
-			retv = 0;
-			break;
-		}
-
-		dev = dev_get_by_index(net, ifindex);
-		retv = -EADDRNOTAVAIL;
-		if (!dev)
-			break;
-		dev_put(dev);
-
-		retv = -EINVAL;
-		if (sk->sk_bound_dev_if)
-			break;
-
-		np->ucast_oif = ifindex;
-		retv = 0;
-		break;
-	}
-
 	case IPV6_ADD_MEMBERSHIP:
 	case IPV6_DROP_MEMBERSHIP:
 	{
@@ -1369,7 +1363,7 @@ int do_ipv6_getsockopt(struct sock *sk, int level, int optname,
 		break;
 
 	case IPV6_UNICAST_IF:
-		val = (__force int)htonl((__u32) np->ucast_oif);
+		val = (__force int)htonl((__u32) READ_ONCE(np->ucast_oif));
 		break;
 
 	case IPV6_MTU_DISCOVER:
diff --git a/net/ipv6/ping.c b/net/ipv6/ping.c
index 465e8d0040671f689e0e5e1f24c024c356ce0fd1..ef2059c889554aaae237ed2cddca0b5402c77bbb 100644
--- a/net/ipv6/ping.c
+++ b/net/ipv6/ping.c
@@ -109,7 +109,7 @@ static int ping_v6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	if (!oif && ipv6_addr_is_multicast(daddr))
 		oif = READ_ONCE(np->mcast_oif);
 	else if (!oif)
-		oif = np->ucast_oif;
+		oif = READ_ONCE(np->ucast_oif);
 
 	addr_type = ipv6_addr_type(daddr);
 	if ((__ipv6_addr_needs_scope_id(addr_type) && !oif) ||
@@ -159,7 +159,7 @@ static int ping_v6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	if (!fl6.flowi6_oif && ipv6_addr_is_multicast(&fl6.daddr))
 		fl6.flowi6_oif = READ_ONCE(np->mcast_oif);
 	else if (!fl6.flowi6_oif)
-		fl6.flowi6_oif = np->ucast_oif;
+		fl6.flowi6_oif = READ_ONCE(np->ucast_oif);
 
 	pfh.icmph.type = user_icmph.icmp6_type;
 	pfh.icmph.code = user_icmph.icmp6_code;
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 59a1e269a82c1af6eb73570ef7a43e0f0f61ab80..03dbb874c363bfa796631c1c3cc49895aae56c5a 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -878,7 +878,7 @@ static int rawv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	if (!fl6.flowi6_oif && ipv6_addr_is_multicast(&fl6.daddr))
 		fl6.flowi6_oif = READ_ONCE(np->mcast_oif);
 	else if (!fl6.flowi6_oif)
-		fl6.flowi6_oif = np->ucast_oif;
+		fl6.flowi6_oif = READ_ONCE(np->ucast_oif);
 	security_sk_classify_flow(sk, flowi6_to_flowi_common(&fl6));
 
 	if (hdrincl)
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 0b7c755faa77b1ddd4feb5dea185f6dd7be45091..594e3f23c12909fe6f245bf31057278169cd85c5 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1544,7 +1544,7 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 		fl6->flowi6_oif = READ_ONCE(np->mcast_oif);
 		connected = false;
 	} else if (!fl6->flowi6_oif)
-		fl6->flowi6_oif = np->ucast_oif;
+		fl6->flowi6_oif = READ_ONCE(np->ucast_oif);
 
 	security_sk_classify_flow(sk, flowi6_to_flowi_common(fl6));
 
diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 17301f9dd228db80be1bdc3cb858ac41d5268e36..dd3153966173db09d42de02fa3ad4d44d05620f4 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -601,7 +601,7 @@ static int l2tp_ip6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	if (!fl6.flowi6_oif && ipv6_addr_is_multicast(&fl6.daddr))
 		fl6.flowi6_oif = READ_ONCE(np->mcast_oif);
 	else if (!fl6.flowi6_oif)
-		fl6.flowi6_oif = np->ucast_oif;
+		fl6.flowi6_oif = READ_ONCE(np->ucast_oif);
 
 	security_sk_classify_flow(sk, flowi6_to_flowi_common(&fl6));
 
-- 
2.43.0.472.g3155946c3a-goog


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ