All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] fix reuseaddr regression
@ 2017-09-23  0:20 Josef Bacik
  2017-09-23  0:20 ` [PATCH 1/3] net: set tb->fast_sk_family Josef Bacik
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Josef Bacik @ 2017-09-23  0:20 UTC (permalink / raw)
  To: davem, netdev, kernel-team, linux-kernel

I introduced a regression when reworking the fastreuse port stuff that allows
bind conflicts to occur once a reuseaddr successfully opens on an existing tb.
The root cause is I reversed an if statement which caused us to set the tb as if
there were no owners on the socket if there were, which obviously is not
correct.

Dave could you please queue these changes up for -stable, I've run them through
the net tests and added another test to check for this problem specifically.
Thanks,

Josef

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/3] net: set tb->fast_sk_family
  2017-09-23  0:20 [PATCH 0/3] fix reuseaddr regression Josef Bacik
@ 2017-09-23  0:20 ` Josef Bacik
  2017-09-23  0:20 ` [PATCH 2/3] net: use inet6_rcv_saddr to compare sockets Josef Bacik
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Josef Bacik @ 2017-09-23  0:20 UTC (permalink / raw)
  To: davem, netdev, kernel-team, linux-kernel; +Cc: Josef Bacik

From: Josef Bacik <jbacik@fb.com>

We need to set the tb->fast_sk_family properly so we can use the proper
comparison function for all subsequent reuseport bind requests.

Fixes: 637bc8bbe6c0 ("inet: reset tb->fastreuseport when adding a reuseport sk")
Reported-and-tested-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 net/ipv4/inet_connection_sock.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index b9c64b40a83a..f87f4805e244 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -328,6 +328,7 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
 			tb->fastuid = uid;
 			tb->fast_rcv_saddr = sk->sk_rcv_saddr;
 			tb->fast_ipv6_only = ipv6_only_sock(sk);
+			tb->fast_sk_family = sk->sk_family;
 #if IS_ENABLED(CONFIG_IPV6)
 			tb->fast_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
 #endif
@@ -354,6 +355,7 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
 				tb->fastuid = uid;
 				tb->fast_rcv_saddr = sk->sk_rcv_saddr;
 				tb->fast_ipv6_only = ipv6_only_sock(sk);
+				tb->fast_sk_family = sk->sk_family;
 #if IS_ENABLED(CONFIG_IPV6)
 				tb->fast_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
 #endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/3] net: use inet6_rcv_saddr to compare sockets
  2017-09-23  0:20 [PATCH 0/3] fix reuseaddr regression Josef Bacik
  2017-09-23  0:20 ` [PATCH 1/3] net: set tb->fast_sk_family Josef Bacik
@ 2017-09-23  0:20 ` Josef Bacik
  2017-09-23  0:20 ` [PATCH 3/3] inet: fix improper empty comparison Josef Bacik
  2017-09-23  3:40 ` [PATCH 0/3] fix reuseaddr regression David Miller
  3 siblings, 0 replies; 6+ messages in thread
From: Josef Bacik @ 2017-09-23  0:20 UTC (permalink / raw)
  To: davem, netdev, kernel-team, linux-kernel; +Cc: Josef Bacik

From: Josef Bacik <jbacik@fb.com>

In ipv6_rcv_saddr_equal() we need to use inet6_rcv_saddr(sk) for the
ipv6 compare with the fast socket information to make sure we're doing
the proper comparisons.

Fixes: 637bc8bbe6c0 ("inet: reset tb->fastreuseport when adding a reuseport sk")
Reported-and-tested-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 net/ipv4/inet_connection_sock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index f87f4805e244..a1bf30438bc5 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -266,7 +266,7 @@ static inline int sk_reuseport_match(struct inet_bind_bucket *tb,
 #if IS_ENABLED(CONFIG_IPV6)
 	if (tb->fast_sk_family == AF_INET6)
 		return ipv6_rcv_saddr_equal(&tb->fast_v6_rcv_saddr,
-					    &sk->sk_v6_rcv_saddr,
+					    inet6_rcv_saddr(sk),
 					    tb->fast_rcv_saddr,
 					    sk->sk_rcv_saddr,
 					    tb->fast_ipv6_only,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 3/3] inet: fix improper empty comparison
  2017-09-23  0:20 [PATCH 0/3] fix reuseaddr regression Josef Bacik
  2017-09-23  0:20 ` [PATCH 1/3] net: set tb->fast_sk_family Josef Bacik
  2017-09-23  0:20 ` [PATCH 2/3] net: use inet6_rcv_saddr to compare sockets Josef Bacik
@ 2017-09-23  0:20 ` Josef Bacik
  2017-09-23  3:40 ` [PATCH 0/3] fix reuseaddr regression David Miller
  3 siblings, 0 replies; 6+ messages in thread
From: Josef Bacik @ 2017-09-23  0:20 UTC (permalink / raw)
  To: davem, netdev, kernel-team, linux-kernel; +Cc: Josef Bacik

From: Josef Bacik <jbacik@fb.com>

When doing my reuseport rework I screwed up and changed a

if (hlist_empty(&tb->owners))

to

if (!hlist_empty(&tb->owners))

This is obviously bad as all of the reuseport/reuse logic was reversed,
which caused weird problems like allowing an ipv4 bind conflict if we
opened an ipv4 only socket on a port followed by an ipv6 only socket on
the same port.

Fixes: b9470c27607b ("inet: kill smallest_size and smallest_port")
Reported-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 net/ipv4/inet_connection_sock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index a1bf30438bc5..c039c937ba90 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -321,7 +321,7 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
 			goto fail_unlock;
 	}
 success:
-	if (!hlist_empty(&tb->owners)) {
+	if (hlist_empty(&tb->owners)) {
 		tb->fastreuse = reuse;
 		if (sk->sk_reuseport) {
 			tb->fastreuseport = FASTREUSEPORT_ANY;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] fix reuseaddr regression
  2017-09-23  0:20 [PATCH 0/3] fix reuseaddr regression Josef Bacik
                   ` (2 preceding siblings ...)
  2017-09-23  0:20 ` [PATCH 3/3] inet: fix improper empty comparison Josef Bacik
@ 2017-09-23  3:40 ` David Miller
  3 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2017-09-23  3:40 UTC (permalink / raw)
  To: josef; +Cc: netdev, kernel-team, linux-kernel

From: Josef Bacik <josef@toxicpanda.com>
Date: Fri, 22 Sep 2017 20:20:05 -0400

> I introduced a regression when reworking the fastreuse port stuff that allows
> bind conflicts to occur once a reuseaddr successfully opens on an existing tb.
> The root cause is I reversed an if statement which caused us to set the tb as if
> there were no owners on the socket if there were, which obviously is not
> correct.
> 
> Dave could you please queue these changes up for -stable, I've run them through
> the net tests and added another test to check for this problem specifically.

Series applied and queued up for -stable, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/3] net: set tb->fast_sk_family
  2017-09-18 16:28 josef
@ 2017-09-18 16:28 ` josef
  0 siblings, 0 replies; 6+ messages in thread
From: josef @ 2017-09-18 16:28 UTC (permalink / raw)
  To: davem, netdev, linux-kernel, crobinso, labbott, kernel-team
  Cc: Josef Bacik, stable

From: Josef Bacik <jbacik@fb.com>

We need to set the tb->fast_sk_family properly so we can use the proper
comparison function for all subsequent reuseport bind requests.

Cc: stable@vger.kernel.org
Fixes: 637bc8bbe6c0 ("inet: reset tb->fastreuseport when adding a reuseport sk")
Reported-and-tested-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 net/ipv4/inet_connection_sock.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index b9c64b40a83a..f87f4805e244 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -328,6 +328,7 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
 			tb->fastuid = uid;
 			tb->fast_rcv_saddr = sk->sk_rcv_saddr;
 			tb->fast_ipv6_only = ipv6_only_sock(sk);
+			tb->fast_sk_family = sk->sk_family;
 #if IS_ENABLED(CONFIG_IPV6)
 			tb->fast_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
 #endif
@@ -354,6 +355,7 @@ int inet_csk_get_port(struct sock *sk, unsigned short snum)
 				tb->fastuid = uid;
 				tb->fast_rcv_saddr = sk->sk_rcv_saddr;
 				tb->fast_ipv6_only = ipv6_only_sock(sk);
+				tb->fast_sk_family = sk->sk_family;
 #if IS_ENABLED(CONFIG_IPV6)
 				tb->fast_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
 #endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-09-23  3:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-23  0:20 [PATCH 0/3] fix reuseaddr regression Josef Bacik
2017-09-23  0:20 ` [PATCH 1/3] net: set tb->fast_sk_family Josef Bacik
2017-09-23  0:20 ` [PATCH 2/3] net: use inet6_rcv_saddr to compare sockets Josef Bacik
2017-09-23  0:20 ` [PATCH 3/3] inet: fix improper empty comparison Josef Bacik
2017-09-23  3:40 ` [PATCH 0/3] fix reuseaddr regression David Miller
  -- strict thread matches above, loose matches on Subject: below --
2017-09-18 16:28 josef
2017-09-18 16:28 ` [PATCH 1/3] net: set tb->fast_sk_family josef

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.