regressions.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [REGRESSION] v6.1+ bind() does not fail with EADDRINUSE if dual stack is bound
@ 2023-03-10 16:01 Paul Holzinger
  2023-03-10 21:25 ` Kuniyuki Iwashima
  2023-04-05 13:31 ` Linux regression tracking #adding (Thorsten Leemhuis)
  0 siblings, 2 replies; 4+ messages in thread
From: Paul Holzinger @ 2023-03-10 16:01 UTC (permalink / raw)
  To: stable; +Cc: netdev, regressions, martin.lau, kuba

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.


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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [REGRESSION] v6.1+ bind() does not fail with EADDRINUSE if dual stack is bound
  2023-03-10 16:01 [REGRESSION] v6.1+ bind() does not fail with EADDRINUSE if dual stack is bound Paul Holzinger
@ 2023-03-10 21:25 ` Kuniyuki Iwashima
  2023-03-10 22:37   ` Kuniyuki Iwashima
  2023-04-05 13:31 ` Linux regression tracking #adding (Thorsten Leemhuis)
  1 sibling, 1 reply; 4+ messages in thread
From: Kuniyuki Iwashima @ 2023-03-10 21:25 UTC (permalink / raw)
  To: pholzing; +Cc: kuba, martin.lau, netdev, regressions, stable, kuniyu

From:   Paul Holzinger <pholzing@redhat.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'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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [REGRESSION] v6.1+ bind() does not fail with EADDRINUSE if dual stack is bound
  2023-03-10 21:25 ` Kuniyuki Iwashima
@ 2023-03-10 22:37   ` Kuniyuki Iwashima
  0 siblings, 0 replies; 4+ messages in thread
From: Kuniyuki Iwashima @ 2023-03-10 22:37 UTC (permalink / raw)
  To: kuniyu; +Cc: kuba, martin.lau, netdev, pholzing, regressions, stable

From:   Kuniyuki Iwashima <kuniyu@amazon.com>
Date:   Fri, 10 Mar 2023 13:25:47 -0800
> From:   Paul Holzinger <pholzing@redhat.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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [REGRESSION] v6.1+ bind() does not fail with EADDRINUSE if dual stack is bound
  2023-03-10 16:01 [REGRESSION] v6.1+ bind() does not fail with EADDRINUSE if dual stack is bound Paul Holzinger
  2023-03-10 21:25 ` Kuniyuki Iwashima
@ 2023-04-05 13:31 ` Linux regression tracking #adding (Thorsten Leemhuis)
  1 sibling, 0 replies; 4+ messages in thread
From: Linux regression tracking #adding (Thorsten Leemhuis) @ 2023-04-05 13:31 UTC (permalink / raw)
  To: Paul Holzinger, stable; +Cc: netdev, regressions, martin.lau, kuba

[TLDR: This mail in primarily relevant for Linux kernel regression
tracking. See link in footer if these mails annoy you.]

On 10.03.23 17:01, Paul Holzinger wrote:
> 
> 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).
> [...]
> Original report: https://github.com/containers/podman/issues/17719
> 
> #regzbot introduced: 5456262d2baa

I had missed that a fix for this was applied, as it didn't contain a
link to the reports for this issue, hence I have to specify it manually
to resolve this:

#regzbot fix: d9ba9934285514f1f9
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-04-05 13:32 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-10 16:01 [REGRESSION] v6.1+ bind() does not fail with EADDRINUSE if dual stack is bound Paul Holzinger
2023-03-10 21:25 ` Kuniyuki Iwashima
2023-03-10 22:37   ` Kuniyuki Iwashima
2023-04-05 13:31 ` Linux regression tracking #adding (Thorsten Leemhuis)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).