| 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
| ||
|
Message-ID: <20250228002953.68029-1-kuniyu@amazon.com>
Date: Thu, 27 Feb 2025 16:29:53 -0800
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <menglong8.dong@...il.com>
CC: <davem@...emloft.net>, <dongml2@...natelecom.cn>, <dsahern@...nel.org>,
<edumazet@...gle.com>, <horms@...nel.org>, <kerneljasonxing@...il.com>,
<kuba@...nel.org>, <kuniyu@...zon.com>, <linux-kernel@...r.kernel.org>,
<ncardwell@...gle.com>, <netdev@...r.kernel.org>, <pabeni@...hat.com>,
<petrm@...dia.com>, <yyd@...gle.com>
Subject: Re: [RFC PATCH net-next] net: ip: add sysctl_ip_reuse_exact_match
From: Menglong Dong <menglong8.dong@...il.com>
Date: Thu, 27 Feb 2025 20:31:37 +0800
> For now, the socket lookup will terminate if the socket is reuse port in
> inet_lhash2_lookup(), which makes the socket is not the best match.
>
> For example, we have socket1 listening on "0.0.0.0:1234" and socket2
> listening on "192.168.1.1:1234", and both of them enabled reuse port. The
> socket1 will always be matched when a connection with the peer ip
> "192.168.1.xx" comes if the socket1 is created later than socket2. This
> is not expected, as socket2 has higher priority.
>
> This can cause unexpected behavior if TCP MD5 keys is used, as described
> in Documentation/networking/vrf.rst -> Applications.
Could you provide a minimal repro setup ?
I somehow fail to reproduce the issue.
> Introduce the sysctl_ip_reuse_exact_match to make it find a best matched
> socket when reuse port is used.
I think we should not introduce a new sysctl knob and an extra lookup,
rather we can solve that in __inet_hash() taking d894ba18d4e4
("soreuseport: fix ordering for mixed v4/v6 sockets") further.
Could you test this patch ?
---8<---
$ git show
commit 4dbc44a153afe51a2b2698a55213e625a02e23c8
Author: Kuniyuki Iwashima <kuniyu@...zon.com>
Date: Thu Feb 27 19:53:43 2025 +0000
tcp: Place non-wildcard sockets before wildcard ones in lhash2.
Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
index 5eea47f135a4..115248bfe463 100644
--- a/include/net/inet_hashtables.h
+++ b/include/net/inet_hashtables.h
@@ -136,6 +136,9 @@ struct inet_bind_hashbucket {
struct inet_listen_hashbucket {
spinlock_t lock;
struct hlist_nulls_head nulls_head;
+#if IS_ENABLED(CONFIG_IPV6)
+ struct hlist_nulls_node *wildcard_node;
+#endif
};
/* This is for listening sockets, thus all sockets which possess wildcards. */
diff --git a/include/net/sock.h b/include/net/sock.h
index efc031163c33..4e8e10d2067b 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -863,6 +863,16 @@ static inline void __sk_nulls_add_node_tail_rcu(struct sock *sk, struct hlist_nu
hlist_nulls_add_tail_rcu(&sk->sk_nulls_node, list);
}
+static inline void __sk_nulls_add_node_before_rcu(struct sock *sk, struct hlist_nulls_node *next)
+{
+ struct hlist_nulls_node *n = &sk->sk_nulls_node;
+
+ WRITE_ONCE(n->pprev, next->pprev);
+ WRITE_ONCE(n->next, next);
+ WRITE_ONCE(next->pprev, &n->next);
+ WRITE_ONCE(*(n->pprev), n);
+}
+
static inline void sk_nulls_add_node_rcu(struct sock *sk, struct hlist_nulls_head *list)
{
sock_hold(sk);
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index ecda4c65788c..acfb693bb1d4 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -729,6 +729,7 @@ int __inet_hash(struct sock *sk, struct sock *osk)
{
struct inet_hashinfo *hashinfo = tcp_or_dccp_get_hashinfo(sk);
struct inet_listen_hashbucket *ilb2;
+ bool add_tail = false;
int err = 0;
if (sk->sk_state != TCP_LISTEN) {
@@ -737,21 +738,47 @@ int __inet_hash(struct sock *sk, struct sock *osk)
local_bh_enable();
return 0;
}
+
WARN_ON(!sk_unhashed(sk));
ilb2 = inet_lhash2_bucket_sk(hashinfo, sk);
spin_lock(&ilb2->lock);
+
if (sk->sk_reuseport) {
err = inet_reuseport_add_sock(sk, ilb2);
if (err)
goto unlock;
+
+ if (inet_rcv_saddr_any(sk))
+ add_tail = true;
}
+
sock_set_flag(sk, SOCK_RCU_FREE);
- if (IS_ENABLED(CONFIG_IPV6) && sk->sk_reuseport &&
- sk->sk_family == AF_INET6)
- __sk_nulls_add_node_tail_rcu(sk, &ilb2->nulls_head);
- else
- __sk_nulls_add_node_rcu(sk, &ilb2->nulls_head);
+
+#if IS_ENABLED(CONFIG_IPV6)
+ if (sk->sk_family == AF_INET6) {
+ if (add_tail || !ilb2->wildcard_node)
+ __sk_nulls_add_node_tail_rcu(sk, &ilb2->nulls_head);
+ else
+ __sk_nulls_add_node_before_rcu(sk, ilb2->wildcard_node);
+ } else
+#endif
+ {
+ if (!add_tail)
+ __sk_nulls_add_node_rcu(sk, &ilb2->nulls_head);
+#if IS_ENABLED(CONFIG_IPV6)
+ else if (ilb2->wildcard_node)
+ __sk_nulls_add_node_before_rcu(sk, ilb2->wildcard_node);
+#endif
+ else
+ __sk_nulls_add_node_tail_rcu(sk, &ilb2->nulls_head);
+ }
+
+#if IS_ENABLED(CONFIG_IPV6)
+ if (add_tail && (sk->sk_family == AF_INET || !ilb2->wildcard_node))
+ ilb2->wildcard_node = &sk->sk_nulls_node;
+#endif
+
sock_prot_inuse_add(sock_net(sk), sk->sk_prot, 1);
unlock:
spin_unlock(&ilb2->lock);
@@ -794,6 +821,15 @@ void inet_unhash(struct sock *sk)
if (rcu_access_pointer(sk->sk_reuseport_cb))
reuseport_stop_listen_sock(sk);
+#if IS_ENABLED(CONFIG_IPV6)
+ if (&sk->sk_nulls_node == ilb2->wildcard_node) {
+ if (is_a_nulls(sk->sk_nulls_node.next))
+ ilb2->wildcard_node = NULL;
+ else
+ ilb2->wildcard_node = sk->sk_nulls_node.next;
+ }
+#endif
+
__sk_nulls_del_node_init_rcu(sk);
sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1);
spin_unlock(&ilb2->lock);
@@ -1183,6 +1219,9 @@ static void init_hashinfo_lhash2(struct inet_hashinfo *h)
spin_lock_init(&h->lhash2[i].lock);
INIT_HLIST_NULLS_HEAD(&h->lhash2[i].nulls_head,
i + LISTENING_NULLS_BASE);
+#if IS_ENABLED(CONFIG_IPV6)
+ h->lhash2[i].wildcard_node = NULL;
+#endif
}
}
---8<---
Powered by blists - more mailing lists