netdev - Re: [PATCH v1 net 1/5] tcp: Fix bind() regression for v4-mapped-v6 wildcard address.

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20230911200543.78783-1-kuniyu@amazon.com>
Date: Mon, 11 Sep 2023 13:05:43 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <avagin@...gle.com>
CC: <davem@...emloft.net>, <dsahern@...nel.org>, <edumazet@...gle.com>,
	<joannelkoong@...il.com>, <kuba@...nel.org>, <kuni1840@...il.com>,
	<kuniyu@...zon.com>, <netdev@...r.kernel.org>, <pabeni@...hat.com>
Subject: Re: [PATCH v1 net 1/5] tcp: Fix bind() regression for v4-mapped-v6 wildcard address.

From: Andrei Vagin <avagin@...gle.com>
Date: Mon, 11 Sep 2023 13:00:19 -0700
> On Mon, Sep 11, 2023 at 9:52 AM Kuniyuki Iwashima <kuniyu@...zon.com> wrote:
> >
> > Andrei Vagin reported bind() regression with strace logs.
> >
> > If we bind() a TCPv6 socket to ::FFFF:0.0.0.0 and then bind() a TCPv4
> > socket to 127.0.0.1, the 2nd bind() should fail but now succeeds.
> >
> >   from socket import *
> >
> >   s1 = socket(AF_INET6, SOCK_STREAM)
> >   s1.bind(('::ffff:0.0.0.0', 0))
> >
> >   s2 = socket(AF_INET, SOCK_STREAM)
> >   s2.bind(('127.0.0.1', s1.getsockname()[1]))
> >
> > During the 2nd bind(), if tb->family is AF_INET6 and sk->sk_family is
> > AF_INET in inet_bind2_bucket_match_addr_any(), we still need to check
> > if tb has the v4-mapped-v6 wildcard address.
> >
> > The example above does not work after commit 5456262d2baa ("net: Fix
> > incorrect address comparison when searching for a bind2 bucket"), but
> > the blamed change is not the commit.
> >
> > Before the commit, the leading zeros of ::FFFF:0.0.0.0 were treated
> > as 0.0.0.0, and the sequence above worked by chance.  Technically, this
> > case has been broken since bhash2 was introduced.
> >
> > Note that if we bind() two sockets to 127.0.0.1 and then ::FFFF:0.0.0.0,
> > the 2nd bind() fails properly because we fall back to using bhash to
> > detect conflicts for the v4-mapped-v6 address.
> 
> I think we have one more issue here:
> 
> socket(AF_INET6, SOCK_STREAM|SOCK_CLOEXEC, IPPROTO_IP) = 3
> socket(AF_INET, SOCK_STREAM|SOCK_CLOEXEC, IPPROTO_IP) = 4
> bind(3, {sa_family=AF_INET6, sin6_port=htons(9999),
> sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "::ffff:127.0.0.1",
> &sin6_addr), sin6_scope_id=0}, 28) = 0
> bind(4, {sa_family=AF_INET, sin_port=htons(9999),
> sin_addr=inet_addr("127.0.0.1")}, 16) = 0
> 
> I think the second bind has to return EADDRINUSE, doesn't it?

Correct, and the following patch fixes it separately.
Sorry, I forgot to add you in CC of the entire series.
https://lore.kernel.org/netdev/20230911183700.60878-4-kuniyu@amazon.com/