[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230310223752.31024-1-kuniyu@amazon.com>
Date: Fri, 10 Mar 2023 14:37:52 -0800
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <kuniyu@...zon.com>
CC: <kuba@...nel.org>, <martin.lau@...nel.org>,
<netdev@...r.kernel.org>, <pholzing@...hat.com>,
<regressions@...ts.linux.dev>, <stable@...r.kernel.org>
Subject: Re: [REGRESSION] v6.1+ bind() does not fail with EADDRINUSE if dual stack is bound
From: Kuniyuki Iwashima <kuniyu@...zon.com>
Date: Fri, 10 Mar 2023 13:25:47 -0800
> From: Paul Holzinger <pholzing@...hat.com>
> Date: Fri, 10 Mar 2023 17:01:31 +0100
> > Hi all,
> >
> > there seems to be a regression which allows you to bind the same port
> > twice when the first bind call bound to all ip addresses (i. e. dual stack).
> >
> > A second bind call for the same port will succeed if you try to bind to
> > a specific ipv4 (e. g. 127.0.0.1), binding to 0.0.0.0 or an ipv6 address
> > fails correctly with EADDRINUSE.
> >
> > I included a small c program below to show the issue. Normally the
> > second bind call should fail, this was the case before v6.1.
> >
> >
> > I bisected the regression to commit 5456262d2baa ("net: Fix incorrect
> > address comparison when searching for a bind2 bucket").
> >
> > I also checked that the issue is still present in v6.3-rc1.
>
> Thanks for the detailed report.
>
> It seems we should take care of the special case in
> inet_bind2_bucket_match_addr_any().
I confimed this change fixes the regression.
I'll check other paths that 5456262d2baa touched.
---8<---
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index e41fdc38ce19..62c5f7501571 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -828,8 +828,15 @@ bool inet_bind2_bucket_match_addr_any(const struct inet_bind2_bucket *tb, const
#if IS_ENABLED(CONFIG_IPV6)
struct in6_addr addr_any = {};
- if (sk->sk_family != tb->family)
- return false;
+ if (sk->sk_family != tb->family) {
+ if (sk->sk_family == AF_INET6)
+ return net_eq(ib2_net(tb), net) && tb->port == port &&
+ tb->l3mdev == l3mdev && tb->rcv_saddr == 0;
+ else
+ return net_eq(ib2_net(tb), net) && tb->port == port &&
+ tb->l3mdev == l3mdev &&
+ ipv6_addr_equal(&tb->v6_rcv_saddr, &in6addr_any);
+ }
if (sk->sk_family == AF_INET6)
return net_eq(ib2_net(tb), net) && tb->port == port &&
---8<---
Tested:
---8<---
>>> from socket import *
>>>
>>> s = socket(AF_INET6, SOCK_STREAM, 0)
>>> s2 = socket(AF_INET, SOCK_STREAM, 0)
>>>
>>> s.bind(('::', 0))
>>> s
<socket.socket fd=3, family=AddressFamily.AF_INET6, type=SocketKind.SOCK_STREAM, proto=0, laddr=('::', 53147, 0, 0)>
>>>
>>> s2.bind(('0.0.0.0', s.getsockname()[1]))
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
OSError: [Errno 98] Address already in use
>>> s2.bind(('127.0.0.1', s.getsockname()[1]))
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
OSError: [Errno 98] Address already in use
>>>
>>>
>>> s3 = socket(AF_INET, SOCK_STREAM, 0)
>>> s4 = socket(AF_INET6, SOCK_STREAM, 0)
>>>
>>> s3.bind(('0.0.0.0', 0))
>>> s3
<socket.socket fd=5, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=0, laddr=('0.0.0.0', 58359)>
>>>
>>> s4.bind(('::0', s3.getsockname()[1]))
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
OSError: [Errno 98] Address already in use
>>> s4.bind(('::1', s3.getsockname()[1]))
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
OSError: [Errno 98] Address already in use
---8<---
Thanks,
Kuniyuki
>
> I'll fix it.
>
> Thanks,
> Kuniyuki
>
> >
> >
> > Original report: https://github.com/containers/podman/issues/17719
> >
> > #regzbot introduced: 5456262d2baa
> >
> >
> > ```
> >
> > #include <sys/socket.h>
> > #include <sys/un.h>
> > #include <stdlib.h>
> > #include <stdio.h>
> > #include <netinet/in.h>
> > #include <unistd.h>
> >
> > int main(int argc, char *argv[])
> > {
> > int ret, sock1, sock2;
> > struct sockaddr_in6 addr;
> > struct sockaddr_in addr2;
> >
> > sock1 = socket(AF_INET6, SOCK_STREAM, 0);
> > if (sock1 == -1)
> > {
> > perror("socket1");
> > exit(1);
> > }
> > sock2 = socket(AF_INET, SOCK_STREAM, 0);
> > if (sock2 == -1)
> > {
> > perror("socket2");
> > exit(1);
> > }
> >
> > memset(&addr, 0, sizeof(addr));
> > addr.sin6_family = AF_INET6;
> > addr.sin6_addr = in6addr_any;
> > addr.sin6_port = htons(8080);
> >
> > memset(&addr2, 0, sizeof(addr2));
> > addr2.sin_family = AF_INET;
> > addr2.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
> > addr2.sin_port = htons(8080);
> >
> > ret = bind(sock1, (struct sockaddr *)&addr, sizeof(addr));
> > if (ret == -1)
> > {
> > perror("bind1");
> > exit(1);
> > }
> > printf("bind1 ret: %d\n", ret);
> >
> > if ((listen(sock1, 5)) != 0)
> > {
> > perror("listen1");
> > exit(1);
> > }
> >
> > ret = bind(sock2, (struct sockaddr *)&addr2, sizeof(addr2));
> > if (ret == -1)
> > {
> > perror("bind2");
> > exit(1);
> > }
> > printf("bind2 ret: %d\n", ret);
> >
> > if ((listen(sock2, 5)) != 0)
> > {
> > perror("listen2");
> > exit(1);
> > }
> >
> > // uncomment pause() to see with ss -tlpn the bound ports
> > // pause();
> >
> > return 0;
> > }
> >
> > ```
> >
> >
> > Best regards,
> >
> > Paul
Powered by blists - more mailing lists