| Message ID | 20eee0606b06a3e0ec7d90a4cb24a86a1905d4df.1711478269.git.lucien.xin@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: fix the any addr conflict check in inet_bhash2_addr_any_conflict | expand |
From: Xin Long <lucien.xin@gmail.com> Date: Tue, 26 Mar 2024 14:37:49 -0400 > Xiumei reported a socket bind issue with this python script: > > from socket import * > > s_v41 = socket(AF_INET, SOCK_STREAM) > s_v41.bind(('0.0.0.0', 5901)) > > s_v61 = socket(AF_INET6, SOCK_STREAM) > s_v61.setsockopt(IPPROTO_IPV6, IPV6_V6ONLY, 1) > s_v61.bind(('::', 5901)) > > s_v42 = socket(AF_INET, SOCK_STREAM) > s_v42.bind(('localhost', 5901)) > > where s_v42.bind() is expected to fail. Hi, I posted a similar patch yesterday, which needs another round due to build error by another patch though. https://lore.kernel.org/netdev/20240325181923.48769-3-kuniyu@amazon.com/ So, let me repost this series. Thanks! > > However, in this case s_v41 and s_v61 are linked to different buckets and > these buckets are linked into the same bhash2 chain where s_v61's buckets > is ahead of s_v41's. When doing the ANY addr conflict check with s_v42 in > inet_bhash2_addr_any_conflict(), it breaks the bhash2 chain traverse after > matching s_v61 by inet_bind2_bucket_match_addr_any(), but never gets a > chance to match s_v41. Then s_v42.bind() works as ipv6only is set on s_v61 > and inet_bhash2_conflict() returns false. > > This patch fixes the issue by NOT breaking the bhash2 chain traverse until > both inet_bind2_bucket_match_addr_any() and inet_bhash2_conflict() return > true. > > Fixes: 28044fc1d495 ("net: Add a bhash2 table hashed by port and address") > Reported-by: Xiumei Mu <xmu@redhat.com> > Signed-off-by: Xin Long <lucien.xin@gmail.com> > --- > net/ipv4/inet_connection_sock.c | 14 ++++++-------- > 1 file changed, 6 insertions(+), 8 deletions(-) > > diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c > index c038e28e2f1e..a3188f90210b 100644 > --- a/net/ipv4/inet_connection_sock.c > +++ b/net/ipv4/inet_connection_sock.c > @@ -299,14 +299,12 @@ static bool inet_bhash2_addr_any_conflict(const struct sock *sk, int port, int l > > spin_lock(&head2->lock); > > - inet_bind_bucket_for_each(tb2, &head2->chain) > - if (inet_bind2_bucket_match_addr_any(tb2, net, port, l3mdev, sk)) > - break; > - > - if (tb2 && inet_bhash2_conflict(sk, tb2, uid, relax, reuseport_cb_ok, > - reuseport_ok)) { > - spin_unlock(&head2->lock); > - return true; > + inet_bind_bucket_for_each(tb2, &head2->chain) { > + if (inet_bind2_bucket_match_addr_any(tb2, net, port, l3mdev, sk) && > + inet_bhash2_conflict(sk, tb2, uid, relax, reuseport_cb_ok, reuseport_ok)) { > + spin_unlock(&head2->lock); > + return true; > + } > } > > spin_unlock(&head2->lock); > -- > 2.43.0
On Tue, Mar 26, 2024 at 4:36 PM Kuniyuki Iwashima <kuniyu@amazon.com> wrote: > > From: Xin Long <lucien.xin@gmail.com> > Date: Tue, 26 Mar 2024 14:37:49 -0400 > > Xiumei reported a socket bind issue with this python script: > > > > from socket import * > > > > s_v41 = socket(AF_INET, SOCK_STREAM) > > s_v41.bind(('0.0.0.0', 5901)) > > > > s_v61 = socket(AF_INET6, SOCK_STREAM) > > s_v61.setsockopt(IPPROTO_IPV6, IPV6_V6ONLY, 1) > > s_v61.bind(('::', 5901)) > > > > s_v42 = socket(AF_INET, SOCK_STREAM) > > s_v42.bind(('localhost', 5901)) > > > > where s_v42.bind() is expected to fail. > > Hi, > > I posted a similar patch yesterday, which needs another round due to > build error by another patch though. > https://lore.kernel.org/netdev/20240325181923.48769-3-kuniyu@amazon.com/ > > So, let me repost this series. Cool, thanks for letting me know. > > Thanks! > > > > > > However, in this case s_v41 and s_v61 are linked to different buckets and > > these buckets are linked into the same bhash2 chain where s_v61's buckets > > is ahead of s_v41's. When doing the ANY addr conflict check with s_v42 in > > inet_bhash2_addr_any_conflict(), it breaks the bhash2 chain traverse after > > matching s_v61 by inet_bind2_bucket_match_addr_any(), but never gets a > > chance to match s_v41. Then s_v42.bind() works as ipv6only is set on s_v61 > > and inet_bhash2_conflict() returns false. > > > > This patch fixes the issue by NOT breaking the bhash2 chain traverse until > > both inet_bind2_bucket_match_addr_any() and inet_bhash2_conflict() return > > true. > > > > Fixes: 28044fc1d495 ("net: Add a bhash2 table hashed by port and address") > > Reported-by: Xiumei Mu <xmu@redhat.com> > > Signed-off-by: Xin Long <lucien.xin@gmail.com> > > --- > > net/ipv4/inet_connection_sock.c | 14 ++++++-------- > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c > > index c038e28e2f1e..a3188f90210b 100644 > > --- a/net/ipv4/inet_connection_sock.c > > +++ b/net/ipv4/inet_connection_sock.c > > @@ -299,14 +299,12 @@ static bool inet_bhash2_addr_any_conflict(const struct sock *sk, int port, int l > > > > spin_lock(&head2->lock); > > > > - inet_bind_bucket_for_each(tb2, &head2->chain) > > - if (inet_bind2_bucket_match_addr_any(tb2, net, port, l3mdev, sk)) > > - break; > > - > > - if (tb2 && inet_bhash2_conflict(sk, tb2, uid, relax, reuseport_cb_ok, > > - reuseport_ok)) { > > - spin_unlock(&head2->lock); > > - return true; > > + inet_bind_bucket_for_each(tb2, &head2->chain) { > > + if (inet_bind2_bucket_match_addr_any(tb2, net, port, l3mdev, sk) && > > + inet_bhash2_conflict(sk, tb2, uid, relax, reuseport_cb_ok, reuseport_ok)) { > > + spin_unlock(&head2->lock); > > + return true; > > + } > > } > > > > spin_unlock(&head2->lock); > > -- > > 2.43.0
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c index c038e28e2f1e..a3188f90210b 100644 --- a/net/ipv4/inet_connection_sock.c +++ b/net/ipv4/inet_connection_sock.c @@ -299,14 +299,12 @@ static bool inet_bhash2_addr_any_conflict(const struct sock *sk, int port, int l spin_lock(&head2->lock); - inet_bind_bucket_for_each(tb2, &head2->chain) - if (inet_bind2_bucket_match_addr_any(tb2, net, port, l3mdev, sk)) - break; - - if (tb2 && inet_bhash2_conflict(sk, tb2, uid, relax, reuseport_cb_ok, - reuseport_ok)) { - spin_unlock(&head2->lock); - return true; + inet_bind_bucket_for_each(tb2, &head2->chain) { + if (inet_bind2_bucket_match_addr_any(tb2, net, port, l3mdev, sk) && + inet_bhash2_conflict(sk, tb2, uid, relax, reuseport_cb_ok, reuseport_ok)) { + spin_unlock(&head2->lock); + return true; + } } spin_unlock(&head2->lock);
Xiumei reported a socket bind issue with this python script: from socket import * s_v41 = socket(AF_INET, SOCK_STREAM) s_v41.bind(('0.0.0.0', 5901)) s_v61 = socket(AF_INET6, SOCK_STREAM) s_v61.setsockopt(IPPROTO_IPV6, IPV6_V6ONLY, 1) s_v61.bind(('::', 5901)) s_v42 = socket(AF_INET, SOCK_STREAM) s_v42.bind(('localhost', 5901)) where s_v42.bind() is expected to fail. However, in this case s_v41 and s_v61 are linked to different buckets and these buckets are linked into the same bhash2 chain where s_v61's buckets is ahead of s_v41's. When doing the ANY addr conflict check with s_v42 in inet_bhash2_addr_any_conflict(), it breaks the bhash2 chain traverse after matching s_v61 by inet_bind2_bucket_match_addr_any(), but never gets a chance to match s_v41. Then s_v42.bind() works as ipv6only is set on s_v61 and inet_bhash2_conflict() returns false. This patch fixes the issue by NOT breaking the bhash2 chain traverse until both inet_bind2_bucket_match_addr_any() and inet_bhash2_conflict() return true. Fixes: 28044fc1d495 ("net: Add a bhash2 table hashed by port and address") Reported-by: Xiumei Mu <xmu@redhat.com> Signed-off-by: Xin Long <lucien.xin@gmail.com> --- net/ipv4/inet_connection_sock.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)