All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/7] insufficient TCP source port randomness
@ 2022-04-27  6:52 Willy Tarreau
  2022-04-27  6:52 ` [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash Willy Tarreau
                   ` (6 more replies)
  0 siblings, 7 replies; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27  6:52 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel, Willy Tarreau

Hi,

In a not-yet published paper, Moshe Kol, Amit Klein, and Yossi Gilad
report being able to accurately identify a client by forcing it to emit
only 40 times more connections than the number of entries in the
table_perturb[] table, which is indexed by hashing the connection tuple.
The current 2^8 setting allows them to perform that attack with only 10k
connections, which is not hard to achieve in a few seconds.

Eric, Amit and I have been working on this for a few weeks now imagining,
testing and eliminating a number of approaches that Amit and his team were
still able to break or that were found to be too risky or too expensive,
and ended up with the simple improvements in this series that resists to
the attack, doesn't degrade the performance, and preserves a reliable port
selection algorithm to avoid connection failures, including the odd/even
port selection preference that allows bind() to always find a port quickly
even under strong connect() stress.

The approach relies on several factors:
  - resalting the hash secret that's used to choose the table_perturb[]
    entry every 10 seconds to eliminate slow attacks and force the
    attacker to forget everything that was learned after this delay.
    This already eliminates most of the problem because if a client
    stays silent for more than 10 seconds there's no link between the
    previous and the next patterns, and 10s isn't yet frequent enough
    to cause too frequent repetition of a same port that may induce a
    connection failure ;

  - adding small random increments to the source port. Previously, a
    random 0 or 1 was added every 16 ports. Now a random 0 to 7 is
    added after each port. This means that with the default 32768-60999
    range, a worst case rollover happens after 1764 connections, and
    an average of 3137. This doesn't stop statistical attacks but
    requires significantly more iterations of the same attack to
    confirm a guess.

  - increasing the table_perturb[] size from 2^8 to 2^16, which Amit
    says will require 2.6 million connections to be attacked with the
    changes above, making it pointless to get a fingerprint that will
    only last 10 seconds. Due to the size, the table was made dynamic.

  - a few minor improvements on the bits used from the hash, to eliminate
    some unfortunate correlations that may possibly have been exploited
    to design future attack models.

These changes were tested under the most extreme conditions, up to
1.1 million connections per second to one and a few targets, showing no
performance regression, and only 2 connection failures within 13 billion,
which is less than 2^-32 and perfectly within usual values.

The series is split into small reviewable changes and was already reviewed
by Amit and Eric.

Regards,
Willy

---
Eric Dumazet (1):
  tcp: resalt the secret every 10 seconds

Willy Tarreau (6):
  secure_seq: return the full 64-bit of the siphash
  tcp: use different parts of the port_offset for index and offset
  tcp: add small random increments to the source port
  tcp: dynamically allocate the perturb table used by source ports
  tcp: increase source port perturb table to 2^16
  tcp: drop the hash_32() part from the index calculation

 include/net/inet_hashtables.h |  2 +-
 include/net/secure_seq.h      |  2 +-
 net/core/secure_seq.c         | 14 ++++++++----
 net/ipv4/inet_hashtables.c    | 40 ++++++++++++++++++++++-------------
 4 files changed, 37 insertions(+), 21 deletions(-)

-- 
2.17.5


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

* [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash
  2022-04-27  6:52 [PATCH net 0/7] insufficient TCP source port randomness Willy Tarreau
@ 2022-04-27  6:52 ` Willy Tarreau
  2022-04-27  9:56   ` kernel test robot
                     ` (2 more replies)
  2022-04-27  6:52 ` [PATCH net 2/7] tcp: use different parts of the port_offset for index and offset Willy Tarreau
                   ` (5 subsequent siblings)
  6 siblings, 3 replies; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27  6:52 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel, Willy Tarreau,
	Jason A . Donenfeld

SipHash replaced MD5 in secure_ipv4_port_ephemeral() via commit
7cd23e5300c1 ("secure_seq: use SipHash in place of MD5"), but the
output remained truncated to 32-bit only. In order to exploit more
bits from the hash, let's make the function return the full 64-bit
of siphash_3u32().

Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Moshe Kol <moshe.kol@mail.huji.ac.il>
Cc: Yossi Gilad <yossi.gilad@mail.huji.ac.il>
Cc: Amit Klein <aksecurity@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 include/net/inet_hashtables.h | 2 +-
 include/net/secure_seq.h      | 2 +-
 net/core/secure_seq.c         | 2 +-
 net/ipv4/inet_hashtables.c    | 6 +++---
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
index f72ec113ae56..98e1ec1a14f0 100644
--- a/include/net/inet_hashtables.h
+++ b/include/net/inet_hashtables.h
@@ -425,7 +425,7 @@ static inline void sk_rcv_saddr_set(struct sock *sk, __be32 addr)
 }
 
 int __inet_hash_connect(struct inet_timewait_death_row *death_row,
-			struct sock *sk, u32 port_offset,
+			struct sock *sk, u64 port_offset,
 			int (*check_established)(struct inet_timewait_death_row *,
 						 struct sock *, __u16,
 						 struct inet_timewait_sock **));
diff --git a/include/net/secure_seq.h b/include/net/secure_seq.h
index d7d2495f83c2..5cea9ed9c773 100644
--- a/include/net/secure_seq.h
+++ b/include/net/secure_seq.h
@@ -4,7 +4,7 @@
 
 #include <linux/types.h>
 
-u32 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport);
+u64 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport);
 u32 secure_ipv6_port_ephemeral(const __be32 *saddr, const __be32 *daddr,
 			       __be16 dport);
 u32 secure_tcp_seq(__be32 saddr, __be32 daddr,
diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
index 9b8443774449..2cdd43a63f64 100644
--- a/net/core/secure_seq.c
+++ b/net/core/secure_seq.c
@@ -142,7 +142,7 @@ u32 secure_tcp_seq(__be32 saddr, __be32 daddr,
 }
 EXPORT_SYMBOL_GPL(secure_tcp_seq);
 
-u32 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport)
+u64 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport)
 {
 	net_secret_init();
 	return siphash_3u32((__force u32)saddr, (__force u32)daddr,
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 17440840a791..09cbad0488ca 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -504,7 +504,7 @@ static int __inet_check_established(struct inet_timewait_death_row *death_row,
 	return -EADDRNOTAVAIL;
 }
 
-static u32 inet_sk_port_offset(const struct sock *sk)
+static u64 inet_sk_port_offset(const struct sock *sk)
 {
 	const struct inet_sock *inet = inet_sk(sk);
 
@@ -734,7 +734,7 @@ EXPORT_SYMBOL_GPL(inet_unhash);
 static u32 table_perturb[1 << INET_TABLE_PERTURB_SHIFT];
 
 int __inet_hash_connect(struct inet_timewait_death_row *death_row,
-		struct sock *sk, u32 port_offset,
+		struct sock *sk, u64 port_offset,
 		int (*check_established)(struct inet_timewait_death_row *,
 			struct sock *, __u16, struct inet_timewait_sock **))
 {
@@ -859,7 +859,7 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 int inet_hash_connect(struct inet_timewait_death_row *death_row,
 		      struct sock *sk)
 {
-	u32 port_offset = 0;
+	u64 port_offset = 0;
 
 	if (!inet_sk(sk)->inet_num)
 		port_offset = inet_sk_port_offset(sk);
-- 
2.17.5


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

* [PATCH net 2/7] tcp: use different parts of the port_offset for index and offset
  2022-04-27  6:52 [PATCH net 0/7] insufficient TCP source port randomness Willy Tarreau
  2022-04-27  6:52 ` [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash Willy Tarreau
@ 2022-04-27  6:52 ` Willy Tarreau
  2022-04-27  6:52 ` [PATCH net 3/7] tcp: resalt the secret every 10 seconds Willy Tarreau
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27  6:52 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel, Willy Tarreau,
	Jason A . Donenfeld

Amit Klein suggests that we use different parts of port_offset for the
table's index and the port offset so that there is no direct relation
between them.

Cc: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Moshe Kol <moshe.kol@mail.huji.ac.il>
Cc: Yossi Gilad <yossi.gilad@mail.huji.ac.il>
Cc: Amit Klein <aksecurity@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 net/ipv4/inet_hashtables.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 09cbad0488ca..747f272da25b 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -777,7 +777,7 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 	net_get_random_once(table_perturb, sizeof(table_perturb));
 	index = hash_32(port_offset, INET_TABLE_PERTURB_SHIFT);
 
-	offset = (READ_ONCE(table_perturb[index]) + port_offset) % remaining;
+	offset = (READ_ONCE(table_perturb[index]) + (port_offset >> 32)) % remaining;
 	/* In first pass we try ports of @low parity.
 	 * inet_csk_get_port() does the opposite choice.
 	 */
-- 
2.17.5


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

* [PATCH net 3/7] tcp: resalt the secret every 10 seconds
  2022-04-27  6:52 [PATCH net 0/7] insufficient TCP source port randomness Willy Tarreau
  2022-04-27  6:52 ` [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash Willy Tarreau
  2022-04-27  6:52 ` [PATCH net 2/7] tcp: use different parts of the port_offset for index and offset Willy Tarreau
@ 2022-04-27  6:52 ` Willy Tarreau
  2022-04-27 15:56   ` Stephen Hemminger
  2022-04-27  6:52 ` [PATCH net 4/7] tcp: add small random increments to the source port Willy Tarreau
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27  6:52 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel

From: Eric Dumazet <edumazet@google.com>

In order to limit the ability for an observer to recognize the source
ports sequence used to contact a set of destinations, we should
periodically shuffle the secret. 10 seconds looks effective enough
without causing particular issues.

Cc: Moshe Kol <moshe.kol@mail.huji.ac.il>
Cc: Yossi Gilad <yossi.gilad@mail.huji.ac.il>
Cc: Amit Klein <aksecurity@gmail.com>
Tested-by: Willy Tarreau <w@1wt.eu>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/secure_seq.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
index 2cdd43a63f64..200ab4686275 100644
--- a/net/core/secure_seq.c
+++ b/net/core/secure_seq.c
@@ -22,6 +22,8 @@
 static siphash_aligned_key_t net_secret;
 static siphash_aligned_key_t ts_secret;
 
+#define EPHEMERAL_PORT_SHUFFLE_PERIOD (10 * HZ)
+
 static __always_inline void net_secret_init(void)
 {
 	net_get_random_once(&net_secret, sizeof(net_secret));
@@ -101,10 +103,12 @@ u32 secure_ipv6_port_ephemeral(const __be32 *saddr, const __be32 *daddr,
 		struct in6_addr saddr;
 		struct in6_addr daddr;
 		__be16 dport;
+		unsigned int timeseed;
 	} __aligned(SIPHASH_ALIGNMENT) combined = {
 		.saddr = *(struct in6_addr *)saddr,
 		.daddr = *(struct in6_addr *)daddr,
-		.dport = dport
+		.dport = dport,
+		.timeseed = jiffies / EPHEMERAL_PORT_SHUFFLE_PERIOD,
 	};
 	net_secret_init();
 	return siphash(&combined, offsetofend(typeof(combined), dport),
@@ -145,8 +149,10 @@ EXPORT_SYMBOL_GPL(secure_tcp_seq);
 u64 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport)
 {
 	net_secret_init();
-	return siphash_3u32((__force u32)saddr, (__force u32)daddr,
-			    (__force u16)dport, &net_secret);
+	return siphash_4u32((__force u32)saddr, (__force u32)daddr,
+			    (__force u16)dport,
+			    jiffies / EPHEMERAL_PORT_SHUFFLE_PERIOD,
+			    &net_secret);
 }
 EXPORT_SYMBOL_GPL(secure_ipv4_port_ephemeral);
 #endif
-- 
2.17.5


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

* [PATCH net 4/7] tcp: add small random increments to the source port
  2022-04-27  6:52 [PATCH net 0/7] insufficient TCP source port randomness Willy Tarreau
                   ` (2 preceding siblings ...)
  2022-04-27  6:52 ` [PATCH net 3/7] tcp: resalt the secret every 10 seconds Willy Tarreau
@ 2022-04-27  6:52 ` Willy Tarreau
  2022-04-27  6:52 ` [PATCH net 5/7] tcp: dynamically allocate the perturb table used by source ports Willy Tarreau
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27  6:52 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel, Willy Tarreau

Here we're randomly adding between 0 and 7 random increments to the
selected source port in order to add some noise in the source port
selection that will make the next port less predictable.

With the default port range of 32768-60999 this means a worst case
reuse scenario of 14116/8=1764 connections between two consecutive
uses of the same port, with an average of 14116/4.5=3137. This code
was stressed at more than 800000 connections per second to a fixed
target with all connections closed by the client using RSTs (worst
condition) and only 2 connections failed among 13 billion, despite
the hash being reseeded every 10 seconds, indicating a perfectly
safe situation.

Cc: Moshe Kol <moshe.kol@mail.huji.ac.il>
Cc: Yossi Gilad <yossi.gilad@mail.huji.ac.il>
Cc: Amit Klein <aksecurity@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 net/ipv4/inet_hashtables.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 747f272da25b..f58c5caf3130 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -831,11 +831,12 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 	return -EADDRNOTAVAIL;
 
 ok:
-	/* If our first attempt found a candidate, skip next candidate
-	 * in 1/16 of cases to add some noise.
+	/* Here we want to add a little bit of randomness to the next source
+	 * port that will be chosen. We use a max() with a random here so that
+	 * on low contention the randomness is maximal and on high contention
+	 * it may be inexistent.
 	 */
-	if (!i && !(prandom_u32() % 16))
-		i = 2;
+	i = max_t(int, i, (prandom_u32() & 7) * 2);
 	WRITE_ONCE(table_perturb[index], READ_ONCE(table_perturb[index]) + i + 2);
 
 	/* Head lock still held and bh's disabled */
-- 
2.17.5


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

* [PATCH net 5/7] tcp: dynamically allocate the perturb table used by source ports
  2022-04-27  6:52 [PATCH net 0/7] insufficient TCP source port randomness Willy Tarreau
                   ` (3 preceding siblings ...)
  2022-04-27  6:52 ` [PATCH net 4/7] tcp: add small random increments to the source port Willy Tarreau
@ 2022-04-27  6:52 ` Willy Tarreau
  2022-04-27  6:52 ` [PATCH net 6/7] tcp: increase source port perturb table to 2^16 Willy Tarreau
  2022-04-27  6:52 ` [PATCH net 7/7] tcp: drop the hash_32() part from the index calculation Willy Tarreau
  6 siblings, 0 replies; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27  6:52 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel, Willy Tarreau

We'll need to further increase the size of this table and it's likely
that at some point its size will not be suitable anymore for a static
table. Let's allocate it on boot from inet_hashinfo2_init(), which is
called from tcp_init().

Cc: Moshe Kol <moshe.kol@mail.huji.ac.il>
Cc: Yossi Gilad <yossi.gilad@mail.huji.ac.il>
Cc: Amit Klein <aksecurity@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 net/ipv4/inet_hashtables.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index f58c5caf3130..d746e5656baf 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -731,7 +731,8 @@ EXPORT_SYMBOL_GPL(inet_unhash);
  * privacy, this only consumes 1 KB of kernel memory.
  */
 #define INET_TABLE_PERTURB_SHIFT 8
-static u32 table_perturb[1 << INET_TABLE_PERTURB_SHIFT];
+#define INET_TABLE_PERTURB_SIZE (1 << INET_TABLE_PERTURB_SHIFT)
+static u32 *table_perturb;
 
 int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 		struct sock *sk, u64 port_offset,
@@ -774,7 +775,8 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 	if (likely(remaining > 1))
 		remaining &= ~1U;
 
-	net_get_random_once(table_perturb, sizeof(table_perturb));
+	net_get_random_once(table_perturb,
+			    INET_TABLE_PERTURB_SIZE * sizeof(*table_perturb));
 	index = hash_32(port_offset, INET_TABLE_PERTURB_SHIFT);
 
 	offset = (READ_ONCE(table_perturb[index]) + (port_offset >> 32)) % remaining;
@@ -910,6 +912,12 @@ void __init inet_hashinfo2_init(struct inet_hashinfo *h, const char *name,
 					    low_limit,
 					    high_limit);
 	init_hashinfo_lhash2(h);
+
+	/* this one is used for source ports of outgoing connections */
+	table_perturb = kmalloc_array(INET_TABLE_PERTURB_SIZE,
+				      sizeof(*table_perturb), GFP_KERNEL);
+	if (!table_perturb)
+		panic("TCP: failed to alloc table_perturb");
 }
 
 int inet_hashinfo2_init_mod(struct inet_hashinfo *h)
-- 
2.17.5


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

* [PATCH net 6/7] tcp: increase source port perturb table to 2^16
  2022-04-27  6:52 [PATCH net 0/7] insufficient TCP source port randomness Willy Tarreau
                   ` (4 preceding siblings ...)
  2022-04-27  6:52 ` [PATCH net 5/7] tcp: dynamically allocate the perturb table used by source ports Willy Tarreau
@ 2022-04-27  6:52 ` Willy Tarreau
  2022-04-27  8:07   ` David Laight
  2022-04-27  6:52 ` [PATCH net 7/7] tcp: drop the hash_32() part from the index calculation Willy Tarreau
  6 siblings, 1 reply; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27  6:52 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel, Willy Tarreau

Moshe Kol, Amit Klein, and Yossi Gilad reported being able to accurately
identify a client by forcing it to emit only 40 times more connections
than there are entries in the table_perturb[] table. The previous two
improvements consisting in resalting the secret every 10s and adding
randomness to each port selection only slightly improved the situation,
and the current value of 2^8 was too small as it's not very difficult
to make a client emit 10k connections in less than 10 seconds.

Thus we're increasing the perturb table from 2^8 to 2^16 so that the the
same precision now requires 2.6M connections, which is more difficult in
this time frame and harder to hide as a background activity. The impact
is that the table now uses 256 kB instead of 1 kB, which could mostly
affect devices making frequent outgoing connections. However such
components usually target a small set of destinations (load balancers,
database clients, perf assessment tools), and in practice only a few
entries will be visited, like before.

A live test at 1 million connections per second showed no performance
difference from the previous value.

Reported-by: Moshe Kol <moshe.kol@mail.huji.ac.il>
Reported-by: Yossi Gilad <yossi.gilad@mail.huji.ac.il>
Reported-by: Amit Klein <aksecurity@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 net/ipv4/inet_hashtables.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index d746e5656baf..f30c50aaf8e2 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -726,11 +726,12 @@ EXPORT_SYMBOL_GPL(inet_unhash);
  * Note that we use 32bit integers (vs RFC 'short integers')
  * because 2^16 is not a multiple of num_ephemeral and this
  * property might be used by clever attacker.
- * RFC claims using TABLE_LENGTH=10 buckets gives an improvement,
- * we use 256 instead to really give more isolation and
- * privacy, this only consumes 1 KB of kernel memory.
+ * RFC claims using TABLE_LENGTH=10 buckets gives an improvement, though
+ * attacks were since demonstrated, thus we use 65536 instead to really
+ * give more isolation and privacy, at the expense of 256kB of kernel
+ * memory.
  */
-#define INET_TABLE_PERTURB_SHIFT 8
+#define INET_TABLE_PERTURB_SHIFT 16
 #define INET_TABLE_PERTURB_SIZE (1 << INET_TABLE_PERTURB_SHIFT)
 static u32 *table_perturb;
 
-- 
2.17.5


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

* [PATCH net 7/7] tcp: drop the hash_32() part from the index calculation
  2022-04-27  6:52 [PATCH net 0/7] insufficient TCP source port randomness Willy Tarreau
                   ` (5 preceding siblings ...)
  2022-04-27  6:52 ` [PATCH net 6/7] tcp: increase source port perturb table to 2^16 Willy Tarreau
@ 2022-04-27  6:52 ` Willy Tarreau
  6 siblings, 0 replies; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27  6:52 UTC (permalink / raw)
  To: netdev
  Cc: David Miller, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel, Willy Tarreau

In commit 190cc82489f4 ("tcp: change source port randomizarion at
connect() time"), the table_perturb[] array was introduced and an
index was taken from the port_offset via hash_32(). But it turns
out that hash_32() performs a multiplication while the input here
comes from the output of SipHash in secure_seq, that is well
distributed enough to avoid the need for yet another hash.

Suggested-by: Amit Klein <aksecurity@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 net/ipv4/inet_hashtables.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index f30c50aaf8e2..a75e66d7eee6 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -778,7 +778,7 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
 
 	net_get_random_once(table_perturb,
 			    INET_TABLE_PERTURB_SIZE * sizeof(*table_perturb));
-	index = hash_32(port_offset, INET_TABLE_PERTURB_SHIFT);
+	index = port_offset & (INET_TABLE_PERTURB_SIZE - 1);
 
 	offset = (READ_ONCE(table_perturb[index]) + (port_offset >> 32)) % remaining;
 	/* In first pass we try ports of @low parity.
-- 
2.17.5


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

* RE: [PATCH net 6/7] tcp: increase source port perturb table to 2^16
  2022-04-27  6:52 ` [PATCH net 6/7] tcp: increase source port perturb table to 2^16 Willy Tarreau
@ 2022-04-27  8:07   ` David Laight
  2022-04-27  8:19     ` Willy Tarreau
  0 siblings, 1 reply; 24+ messages in thread
From: David Laight @ 2022-04-27  8:07 UTC (permalink / raw)
  To: 'Willy Tarreau', netdev
  Cc: David Miller, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel

From: Willy Tarreau
> Sent: 27 April 2022 07:53
> 
> Moshe Kol, Amit Klein, and Yossi Gilad reported being able to accurately
> identify a client by forcing it to emit only 40 times more connections
> than there are entries in the table_perturb[] table. The previous two
> improvements consisting in resalting the secret every 10s and adding
> randomness to each port selection only slightly improved the situation,
> and the current value of 2^8 was too small as it's not very difficult
> to make a client emit 10k connections in less than 10 seconds.
> 
> Thus we're increasing the perturb table from 2^8 to 2^16 so that the the
> same precision now requires 2.6M connections, which is more difficult in
> this time frame and harder to hide as a background activity. The impact
> is that the table now uses 256 kB instead of 1 kB, which could mostly
> affect devices making frequent outgoing connections. However such
> components usually target a small set of destinations (load balancers,
> database clients, perf assessment tools), and in practice only a few
> entries will be visited, like before.

Increasing the table size has a bigger impact on anyone trying
to get the kernel to run in a limited memory footprint.

All these large tables (often hash tables) soon add up.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


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

* Re: [PATCH net 6/7] tcp: increase source port perturb table to 2^16
  2022-04-27  8:07   ` David Laight
@ 2022-04-27  8:19     ` Willy Tarreau
  0 siblings, 0 replies; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27  8:19 UTC (permalink / raw)
  To: David Laight
  Cc: netdev, David Miller, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel

On Wed, Apr 27, 2022 at 08:07:29AM +0000, David Laight wrote:
> From: Willy Tarreau
> > Sent: 27 April 2022 07:53
> > 
> > Moshe Kol, Amit Klein, and Yossi Gilad reported being able to accurately
> > identify a client by forcing it to emit only 40 times more connections
> > than there are entries in the table_perturb[] table. The previous two
> > improvements consisting in resalting the secret every 10s and adding
> > randomness to each port selection only slightly improved the situation,
> > and the current value of 2^8 was too small as it's not very difficult
> > to make a client emit 10k connections in less than 10 seconds.
> > 
> > Thus we're increasing the perturb table from 2^8 to 2^16 so that the the
> > same precision now requires 2.6M connections, which is more difficult in
> > this time frame and harder to hide as a background activity. The impact
> > is that the table now uses 256 kB instead of 1 kB, which could mostly
> > affect devices making frequent outgoing connections. However such
> > components usually target a small set of destinations (load balancers,
> > database clients, perf assessment tools), and in practice only a few
> > entries will be visited, like before.
> 
> Increasing the table size has a bigger impact on anyone trying
> to get the kernel to run in a limited memory footprint.
> 
> All these large tables (often hash tables) soon add up.

We know, and the initial approach that required a significantly larger
table and an extra table per namespace was a no-go. All these are part
of the reasons for the effort it took to refine the solution in order
to avoid such unacceptable costs. The way it was made here makes it easy
to patch the value for small systems if needed and leaves the door open
for a configurable setting in the future if that was estimated necessary
for certain environments.

Willy

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

* Re: [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash
  2022-04-27  6:52 ` [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash Willy Tarreau
@ 2022-04-27  9:56   ` kernel test robot
  2022-04-27 10:07       ` Willy Tarreau
  2022-04-27 17:18   ` Jason A. Donenfeld
  2022-04-28  1:59   ` kernel test robot
  2 siblings, 1 reply; 24+ messages in thread
From: kernel test robot @ 2022-04-27  9:56 UTC (permalink / raw)
  To: Willy Tarreau, netdev
  Cc: kbuild-all, Jakub Kicinski, Eric Dumazet, Moshe Kol, Yossi Gilad,
	Amit Klein, linux-kernel, Willy Tarreau, Jason A . Donenfeld

Hi Willy,

I love your patch! Yet something to improve:

[auto build test ERROR on net/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 71cffebf6358a7f5031f5b208bbdc1cb4db6e539
config: i386-randconfig-r026-20220425 (https://download.01.org/0day-ci/archive/20220427/202204271705.VrWNPv7n-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/01b26e522b598adf346b809075880feab3dcdc08
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
        git checkout 01b26e522b598adf346b809075880feab3dcdc08
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: net/ipv4/inet_hashtables.o: in function `__inet_hash_connect':
>> inet_hashtables.c:(.text+0x187d): undefined reference to `__umoddi3'

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

* Re: [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash
  2022-04-27  9:56   ` kernel test robot
@ 2022-04-27 10:07       ` Willy Tarreau
  0 siblings, 0 replies; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27 10:07 UTC (permalink / raw)
  To: kernel test robot
  Cc: netdev, kbuild-all, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel, Jason A . Donenfeld

On Wed, Apr 27, 2022 at 05:56:41PM +0800, kernel test robot wrote:
> Hi Willy,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on net/master]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 71cffebf6358a7f5031f5b208bbdc1cb4db6e539
> config: i386-randconfig-r026-20220425 (https://download.01.org/0day-ci/archive/20220427/202204271705.VrWNPv7n-lkp@intel.com/config)
> compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
> reproduce (this is a W=1 build):
>         # https://github.com/intel-lab-lkp/linux/commit/01b26e522b598adf346b809075880feab3dcdc08
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
>         git checkout 01b26e522b598adf346b809075880feab3dcdc08
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    ld: net/ipv4/inet_hashtables.o: in function `__inet_hash_connect':
> >> inet_hashtables.c:(.text+0x187d): undefined reference to `__umoddi3'

Argh! indeed, we spoke about using div_u64_rem() at the beginning and
that one vanished over time. Will respin it.

Thanks,
Willy

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

* Re: [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash
@ 2022-04-27 10:07       ` Willy Tarreau
  0 siblings, 0 replies; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27 10:07 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 1601 bytes --]

On Wed, Apr 27, 2022 at 05:56:41PM +0800, kernel test robot wrote:
> Hi Willy,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on net/master]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 71cffebf6358a7f5031f5b208bbdc1cb4db6e539
> config: i386-randconfig-r026-20220425 (https://download.01.org/0day-ci/archive/20220427/202204271705.VrWNPv7n-lkp(a)intel.com/config)
> compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
> reproduce (this is a W=1 build):
>         # https://github.com/intel-lab-lkp/linux/commit/01b26e522b598adf346b809075880feab3dcdc08
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
>         git checkout 01b26e522b598adf346b809075880feab3dcdc08
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    ld: net/ipv4/inet_hashtables.o: in function `__inet_hash_connect':
> >> inet_hashtables.c:(.text+0x187d): undefined reference to `__umoddi3'

Argh! indeed, we spoke about using div_u64_rem() at the beginning and
that one vanished over time. Will respin it.

Thanks,
Willy

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

* Re: [PATCH net 3/7] tcp: resalt the secret every 10 seconds
  2022-04-27  6:52 ` [PATCH net 3/7] tcp: resalt the secret every 10 seconds Willy Tarreau
@ 2022-04-27 15:56   ` Stephen Hemminger
  2022-04-27 16:21     ` Willy Tarreau
  0 siblings, 1 reply; 24+ messages in thread
From: Stephen Hemminger @ 2022-04-27 15:56 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: netdev, David Miller, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel

On Wed, 27 Apr 2022 08:52:29 +0200
Willy Tarreau <w@1wt.eu> wrote:

> From: Eric Dumazet <edumazet@google.com>
> 
> In order to limit the ability for an observer to recognize the source
> ports sequence used to contact a set of destinations, we should
> periodically shuffle the secret. 10 seconds looks effective enough
> without causing particular issues.
> 
> Cc: Moshe Kol <moshe.kol@mail.huji.ac.il>
> Cc: Yossi Gilad <yossi.gilad@mail.huji.ac.il>
> Cc: Amit Klein <aksecurity@gmail.com>
> Tested-by: Willy Tarreau <w@1wt.eu>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  net/core/secure_seq.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
> index 2cdd43a63f64..200ab4686275 100644
> --- a/net/core/secure_seq.c
> +++ b/net/core/secure_seq.c
> @@ -22,6 +22,8 @@
>  static siphash_aligned_key_t net_secret;
>  static siphash_aligned_key_t ts_secret;
>  

Rather than hard coding, why not have a sysctl knob for this?
That way the tinfoil types can set it smaller.

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

* Re: [PATCH net 3/7] tcp: resalt the secret every 10 seconds
  2022-04-27 15:56   ` Stephen Hemminger
@ 2022-04-27 16:21     ` Willy Tarreau
  0 siblings, 0 replies; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27 16:21 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: netdev, David Miller, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel

Hi Stephen,

On Wed, Apr 27, 2022 at 08:56:21AM -0700, Stephen Hemminger wrote:
> On Wed, 27 Apr 2022 08:52:29 +0200
> Willy Tarreau <w@1wt.eu> wrote:
> 
> > From: Eric Dumazet <edumazet@google.com>
> > 
> > In order to limit the ability for an observer to recognize the source
> > ports sequence used to contact a set of destinations, we should
> > periodically shuffle the secret. 10 seconds looks effective enough
> > without causing particular issues.
> > 
> > Cc: Moshe Kol <moshe.kol@mail.huji.ac.il>
> > Cc: Yossi Gilad <yossi.gilad@mail.huji.ac.il>
> > Cc: Amit Klein <aksecurity@gmail.com>
> > Tested-by: Willy Tarreau <w@1wt.eu>
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > ---
> >  net/core/secure_seq.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
> > index 2cdd43a63f64..200ab4686275 100644
> > --- a/net/core/secure_seq.c
> > +++ b/net/core/secure_seq.c
> > @@ -22,6 +22,8 @@
> >  static siphash_aligned_key_t net_secret;
> >  static siphash_aligned_key_t ts_secret;
> >  
> 
> Rather than hard coding, why not have a sysctl knob for this?
> That way the tinfoil types can set it smaller.

It's a legit question. First I think that there's no good value; before
it used to be infinite, and now we're trying to figure a reasonable value
that make the attack impractical without going too close to the risk of
occasionally failing to establish a connection. I'm really not convinced
that there's any benefit in fiddling with that, except for breaking one's
stack by resalting too often and complaining about stupid network issues
with ACK or RST being sent in response to a SYN.

And stupidly, dividing jiffies by a constant known at build time is
slightly cheaper than dividing by a variable. I know it's a detail but
we tried hard to limit the accumulation of details here :-/

Just my two cents,
Willy

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

* Re: [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash
  2022-04-27 10:07       ` Willy Tarreau
@ 2022-04-27 16:35         ` Willy Tarreau
  -1 siblings, 0 replies; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27 16:35 UTC (permalink / raw)
  To: kernel test robot
  Cc: netdev, kbuild-all, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel, Jason A . Donenfeld

On Wed, Apr 27, 2022 at 12:07:14PM +0200, Willy Tarreau wrote:
> On Wed, Apr 27, 2022 at 05:56:41PM +0800, kernel test robot wrote:
> > Hi Willy,
> > 
> > I love your patch! Yet something to improve:
> > 
> > [auto build test ERROR on net/master]
> > 
> > url:    https://github.com/intel-lab-lkp/linux/commits/Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 71cffebf6358a7f5031f5b208bbdc1cb4db6e539
> > config: i386-randconfig-r026-20220425 (https://download.01.org/0day-ci/archive/20220427/202204271705.VrWNPv7n-lkp@intel.com/config)
> > compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
> > reproduce (this is a W=1 build):
> >         # https://github.com/intel-lab-lkp/linux/commit/01b26e522b598adf346b809075880feab3dcdc08
> >         git remote add linux-review https://github.com/intel-lab-lkp/linux
> >         git fetch --no-tags linux-review Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
> >         git checkout 01b26e522b598adf346b809075880feab3dcdc08
> >         # save the config file
> >         mkdir build_dir && cp config build_dir/.config
> >         make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
> > 
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > 
> > All errors (new ones prefixed by >>):
> > 
> >    ld: net/ipv4/inet_hashtables.o: in function `__inet_hash_connect':
> > >> inet_hashtables.c:(.text+0x187d): undefined reference to `__umoddi3'
> 
> Argh! indeed, we spoke about using div_u64_rem() at the beginning and
> that one vanished over time. Will respin it.

I fixed it, built it for i386 and x86_64, tested it on x86_64 and confirmed
that it still does what I need. The change is only this:

-       offset = (READ_ONCE(table_perturb[index]) + (port_offset >> 32)) % remaining;
+       div_u64_rem(READ_ONCE(table_perturb[index]) + (port_offset >> 32), remaining, &offset);

I'll send a v2 series in a few hours if there are no more comments.

Willy

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

* Re: [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash
@ 2022-04-27 16:35         ` Willy Tarreau
  0 siblings, 0 replies; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27 16:35 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2110 bytes --]

On Wed, Apr 27, 2022 at 12:07:14PM +0200, Willy Tarreau wrote:
> On Wed, Apr 27, 2022 at 05:56:41PM +0800, kernel test robot wrote:
> > Hi Willy,
> > 
> > I love your patch! Yet something to improve:
> > 
> > [auto build test ERROR on net/master]
> > 
> > url:    https://github.com/intel-lab-lkp/linux/commits/Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 71cffebf6358a7f5031f5b208bbdc1cb4db6e539
> > config: i386-randconfig-r026-20220425 (https://download.01.org/0day-ci/archive/20220427/202204271705.VrWNPv7n-lkp(a)intel.com/config)
> > compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
> > reproduce (this is a W=1 build):
> >         # https://github.com/intel-lab-lkp/linux/commit/01b26e522b598adf346b809075880feab3dcdc08
> >         git remote add linux-review https://github.com/intel-lab-lkp/linux
> >         git fetch --no-tags linux-review Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
> >         git checkout 01b26e522b598adf346b809075880feab3dcdc08
> >         # save the config file
> >         mkdir build_dir && cp config build_dir/.config
> >         make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
> > 
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > 
> > All errors (new ones prefixed by >>):
> > 
> >    ld: net/ipv4/inet_hashtables.o: in function `__inet_hash_connect':
> > >> inet_hashtables.c:(.text+0x187d): undefined reference to `__umoddi3'
> 
> Argh! indeed, we spoke about using div_u64_rem() at the beginning and
> that one vanished over time. Will respin it.

I fixed it, built it for i386 and x86_64, tested it on x86_64 and confirmed
that it still does what I need. The change is only this:

-       offset = (READ_ONCE(table_perturb[index]) + (port_offset >> 32)) % remaining;
+       div_u64_rem(READ_ONCE(table_perturb[index]) + (port_offset >> 32), remaining, &offset);

I'll send a v2 series in a few hours if there are no more comments.

Willy

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

* Re: [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash
  2022-04-27 16:35         ` Willy Tarreau
@ 2022-04-27 16:50           ` Eric Dumazet
  -1 siblings, 0 replies; 24+ messages in thread
From: Eric Dumazet @ 2022-04-27 16:50 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: kernel test robot, netdev, kbuild-all, Jakub Kicinski, Moshe Kol,
	Yossi Gilad, Amit Klein, LKML, Jason A . Donenfeld

On Wed, Apr 27, 2022 at 9:35 AM Willy Tarreau <w@1wt.eu> wrote:
>
> On Wed, Apr 27, 2022 at 12:07:14PM +0200, Willy Tarreau wrote:
> > On Wed, Apr 27, 2022 at 05:56:41PM +0800, kernel test robot wrote:
> > > Hi Willy,
> > >
> > > I love your patch! Yet something to improve:
> > >
> > > [auto build test ERROR on net/master]
> > >
> > > url:    https://github.com/intel-lab-lkp/linux/commits/Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 71cffebf6358a7f5031f5b208bbdc1cb4db6e539
> > > config: i386-randconfig-r026-20220425 (https://download.01.org/0day-ci/archive/20220427/202204271705.VrWNPv7n-lkp@intel.com/config)
> > > compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
> > > reproduce (this is a W=1 build):
> > >         # https://github.com/intel-lab-lkp/linux/commit/01b26e522b598adf346b809075880feab3dcdc08
> > >         git remote add linux-review https://github.com/intel-lab-lkp/linux
> > >         git fetch --no-tags linux-review Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
> > >         git checkout 01b26e522b598adf346b809075880feab3dcdc08
> > >         # save the config file
> > >         mkdir build_dir && cp config build_dir/.config
> > >         make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
> > >
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kernel test robot <lkp@intel.com>
> > >
> > > All errors (new ones prefixed by >>):
> > >
> > >    ld: net/ipv4/inet_hashtables.o: in function `__inet_hash_connect':
> > > >> inet_hashtables.c:(.text+0x187d): undefined reference to `__umoddi3'
> >
> > Argh! indeed, we spoke about using div_u64_rem() at the beginning and
> > that one vanished over time. Will respin it.
>
> I fixed it, built it for i386 and x86_64, tested it on x86_64 and confirmed
> that it still does what I need. The change is only this:
>
> -       offset = (READ_ONCE(table_perturb[index]) + (port_offset >> 32)) % remaining;
> +       div_u64_rem(READ_ONCE(table_perturb[index]) + (port_offset >> 32), remaining, &offset);
>
> I'll send a v2 series in a few hours if there are no more comments.

We really do not need 33 bits here.

I would suggest using a 32bit divide.

offset = READ_ONCE(table_perturb[index]) + (port_offset >> 32));
offset %= remaining;

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

* Re: [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash
@ 2022-04-27 16:50           ` Eric Dumazet
  0 siblings, 0 replies; 24+ messages in thread
From: Eric Dumazet @ 2022-04-27 16:50 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2407 bytes --]

On Wed, Apr 27, 2022 at 9:35 AM Willy Tarreau <w@1wt.eu> wrote:
>
> On Wed, Apr 27, 2022 at 12:07:14PM +0200, Willy Tarreau wrote:
> > On Wed, Apr 27, 2022 at 05:56:41PM +0800, kernel test robot wrote:
> > > Hi Willy,
> > >
> > > I love your patch! Yet something to improve:
> > >
> > > [auto build test ERROR on net/master]
> > >
> > > url:    https://github.com/intel-lab-lkp/linux/commits/Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 71cffebf6358a7f5031f5b208bbdc1cb4db6e539
> > > config: i386-randconfig-r026-20220425 (https://download.01.org/0day-ci/archive/20220427/202204271705.VrWNPv7n-lkp(a)intel.com/config)
> > > compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
> > > reproduce (this is a W=1 build):
> > >         # https://github.com/intel-lab-lkp/linux/commit/01b26e522b598adf346b809075880feab3dcdc08
> > >         git remote add linux-review https://github.com/intel-lab-lkp/linux
> > >         git fetch --no-tags linux-review Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
> > >         git checkout 01b26e522b598adf346b809075880feab3dcdc08
> > >         # save the config file
> > >         mkdir build_dir && cp config build_dir/.config
> > >         make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
> > >
> > > If you fix the issue, kindly add following tag as appropriate
> > > Reported-by: kernel test robot <lkp@intel.com>
> > >
> > > All errors (new ones prefixed by >>):
> > >
> > >    ld: net/ipv4/inet_hashtables.o: in function `__inet_hash_connect':
> > > >> inet_hashtables.c:(.text+0x187d): undefined reference to `__umoddi3'
> >
> > Argh! indeed, we spoke about using div_u64_rem() at the beginning and
> > that one vanished over time. Will respin it.
>
> I fixed it, built it for i386 and x86_64, tested it on x86_64 and confirmed
> that it still does what I need. The change is only this:
>
> -       offset = (READ_ONCE(table_perturb[index]) + (port_offset >> 32)) % remaining;
> +       div_u64_rem(READ_ONCE(table_perturb[index]) + (port_offset >> 32), remaining, &offset);
>
> I'll send a v2 series in a few hours if there are no more comments.

We really do not need 33 bits here.

I would suggest using a 32bit divide.

offset = READ_ONCE(table_perturb[index]) + (port_offset >> 32));
offset %= remaining;

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

* Re: [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash
  2022-04-27 16:50           ` Eric Dumazet
@ 2022-04-27 16:56             ` Willy Tarreau
  -1 siblings, 0 replies; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27 16:56 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: kernel test robot, netdev, kbuild-all, Jakub Kicinski, Moshe Kol,
	Yossi Gilad, Amit Klein, LKML, Jason A . Donenfeld

On Wed, Apr 27, 2022 at 09:50:06AM -0700, Eric Dumazet wrote:
> On Wed, Apr 27, 2022 at 9:35 AM Willy Tarreau <w@1wt.eu> wrote:
> >
> > On Wed, Apr 27, 2022 at 12:07:14PM +0200, Willy Tarreau wrote:
> > > On Wed, Apr 27, 2022 at 05:56:41PM +0800, kernel test robot wrote:
> > > > Hi Willy,
> > > >
> > > > I love your patch! Yet something to improve:
> > > >
> > > > [auto build test ERROR on net/master]
> > > >
> > > > url:    https://github.com/intel-lab-lkp/linux/commits/Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
> > > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 71cffebf6358a7f5031f5b208bbdc1cb4db6e539
> > > > config: i386-randconfig-r026-20220425 (https://download.01.org/0day-ci/archive/20220427/202204271705.VrWNPv7n-lkp@intel.com/config)
> > > > compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
> > > > reproduce (this is a W=1 build):
> > > >         # https://github.com/intel-lab-lkp/linux/commit/01b26e522b598adf346b809075880feab3dcdc08
> > > >         git remote add linux-review https://github.com/intel-lab-lkp/linux
> > > >         git fetch --no-tags linux-review Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
> > > >         git checkout 01b26e522b598adf346b809075880feab3dcdc08
> > > >         # save the config file
> > > >         mkdir build_dir && cp config build_dir/.config
> > > >         make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
> > > >
> > > > If you fix the issue, kindly add following tag as appropriate
> > > > Reported-by: kernel test robot <lkp@intel.com>
> > > >
> > > > All errors (new ones prefixed by >>):
> > > >
> > > >    ld: net/ipv4/inet_hashtables.o: in function `__inet_hash_connect':
> > > > >> inet_hashtables.c:(.text+0x187d): undefined reference to `__umoddi3'
> > >
> > > Argh! indeed, we spoke about using div_u64_rem() at the beginning and
> > > that one vanished over time. Will respin it.
> >
> > I fixed it, built it for i386 and x86_64, tested it on x86_64 and confirmed
> > that it still does what I need. The change is only this:
> >
> > -       offset = (READ_ONCE(table_perturb[index]) + (port_offset >> 32)) % remaining;
> > +       div_u64_rem(READ_ONCE(table_perturb[index]) + (port_offset >> 32), remaining, &offset);
> >
> > I'll send a v2 series in a few hours if there are no more comments.
> 
> We really do not need 33 bits here.
> 
> I would suggest using a 32bit divide.
> 
> offset = READ_ONCE(table_perturb[index]) + (port_offset >> 32));
> offset %= remaining;

Yeah much better indeed, I'll do that. Thanks Eric!

Willy


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

* Re: [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash
@ 2022-04-27 16:56             ` Willy Tarreau
  0 siblings, 0 replies; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27 16:56 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2632 bytes --]

On Wed, Apr 27, 2022 at 09:50:06AM -0700, Eric Dumazet wrote:
> On Wed, Apr 27, 2022 at 9:35 AM Willy Tarreau <w@1wt.eu> wrote:
> >
> > On Wed, Apr 27, 2022 at 12:07:14PM +0200, Willy Tarreau wrote:
> > > On Wed, Apr 27, 2022 at 05:56:41PM +0800, kernel test robot wrote:
> > > > Hi Willy,
> > > >
> > > > I love your patch! Yet something to improve:
> > > >
> > > > [auto build test ERROR on net/master]
> > > >
> > > > url:    https://github.com/intel-lab-lkp/linux/commits/Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
> > > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 71cffebf6358a7f5031f5b208bbdc1cb4db6e539
> > > > config: i386-randconfig-r026-20220425 (https://download.01.org/0day-ci/archive/20220427/202204271705.VrWNPv7n-lkp(a)intel.com/config)
> > > > compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
> > > > reproduce (this is a W=1 build):
> > > >         # https://github.com/intel-lab-lkp/linux/commit/01b26e522b598adf346b809075880feab3dcdc08
> > > >         git remote add linux-review https://github.com/intel-lab-lkp/linux
> > > >         git fetch --no-tags linux-review Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
> > > >         git checkout 01b26e522b598adf346b809075880feab3dcdc08
> > > >         # save the config file
> > > >         mkdir build_dir && cp config build_dir/.config
> > > >         make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
> > > >
> > > > If you fix the issue, kindly add following tag as appropriate
> > > > Reported-by: kernel test robot <lkp@intel.com>
> > > >
> > > > All errors (new ones prefixed by >>):
> > > >
> > > >    ld: net/ipv4/inet_hashtables.o: in function `__inet_hash_connect':
> > > > >> inet_hashtables.c:(.text+0x187d): undefined reference to `__umoddi3'
> > >
> > > Argh! indeed, we spoke about using div_u64_rem() at the beginning and
> > > that one vanished over time. Will respin it.
> >
> > I fixed it, built it for i386 and x86_64, tested it on x86_64 and confirmed
> > that it still does what I need. The change is only this:
> >
> > -       offset = (READ_ONCE(table_perturb[index]) + (port_offset >> 32)) % remaining;
> > +       div_u64_rem(READ_ONCE(table_perturb[index]) + (port_offset >> 32), remaining, &offset);
> >
> > I'll send a v2 series in a few hours if there are no more comments.
> 
> We really do not need 33 bits here.
> 
> I would suggest using a 32bit divide.
> 
> offset = READ_ONCE(table_perturb[index]) + (port_offset >> 32));
> offset %= remaining;

Yeah much better indeed, I'll do that. Thanks Eric!

Willy

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

* Re: [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash
  2022-04-27  6:52 ` [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash Willy Tarreau
  2022-04-27  9:56   ` kernel test robot
@ 2022-04-27 17:18   ` Jason A. Donenfeld
  2022-04-27 20:19     ` Willy Tarreau
  2022-04-28  1:59   ` kernel test robot
  2 siblings, 1 reply; 24+ messages in thread
From: Jason A. Donenfeld @ 2022-04-27 17:18 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: netdev, David Miller, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel

Hi Willy,

On Wed, Apr 27, 2022 at 08:52:27AM +0200, Willy Tarreau wrote:
> diff --git a/include/net/secure_seq.h b/include/net/secure_seq.h
> index d7d2495f83c2..5cea9ed9c773 100644
> --- a/include/net/secure_seq.h
> +++ b/include/net/secure_seq.h
> @@ -4,7 +4,7 @@
>  
>  #include <linux/types.h>
>  
> -u32 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport);
> +u64 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport);
>  u32 secure_ipv6_port_ephemeral(const __be32 *saddr, const __be32 *daddr,
>  			       __be16 dport);
>  u32 secure_tcp_seq(__be32 saddr, __be32 daddr,
> diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
> index 9b8443774449..2cdd43a63f64 100644
> --- a/net/core/secure_seq.c
> +++ b/net/core/secure_seq.c
> @@ -142,7 +142,7 @@ u32 secure_tcp_seq(__be32 saddr, __be32 daddr,
>  }
>  EXPORT_SYMBOL_GPL(secure_tcp_seq);
>  
> -u32 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport)
> +u64 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport)
>  {
>  	net_secret_init();
>  	return siphash_3u32((__force u32)saddr, (__force u32)daddr,

Should you be doing the same with secure_ipv6_port_ephemeral() too? Why
the asymmetry?

Jason

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

* Re: [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash
  2022-04-27 17:18   ` Jason A. Donenfeld
@ 2022-04-27 20:19     ` Willy Tarreau
  0 siblings, 0 replies; 24+ messages in thread
From: Willy Tarreau @ 2022-04-27 20:19 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: netdev, David Miller, Jakub Kicinski, Eric Dumazet, Moshe Kol,
	Yossi Gilad, Amit Klein, linux-kernel

Hi Jason,

On Wed, Apr 27, 2022 at 07:18:48PM +0200, Jason A. Donenfeld wrote:
> Hi Willy,
> 
> On Wed, Apr 27, 2022 at 08:52:27AM +0200, Willy Tarreau wrote:
> > diff --git a/include/net/secure_seq.h b/include/net/secure_seq.h
> > index d7d2495f83c2..5cea9ed9c773 100644
> > --- a/include/net/secure_seq.h
> > +++ b/include/net/secure_seq.h
> > @@ -4,7 +4,7 @@
> >  
> >  #include <linux/types.h>
> >  
> > -u32 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport);
> > +u64 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport);
> >  u32 secure_ipv6_port_ephemeral(const __be32 *saddr, const __be32 *daddr,
> >  			       __be16 dport);
> >  u32 secure_tcp_seq(__be32 saddr, __be32 daddr,
> > diff --git a/net/core/secure_seq.c b/net/core/secure_seq.c
> > index 9b8443774449..2cdd43a63f64 100644
> > --- a/net/core/secure_seq.c
> > +++ b/net/core/secure_seq.c
> > @@ -142,7 +142,7 @@ u32 secure_tcp_seq(__be32 saddr, __be32 daddr,
> >  }
> >  EXPORT_SYMBOL_GPL(secure_tcp_seq);
> >  
> > -u32 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport)
> > +u64 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport)
> >  {
> >  	net_secret_init();
> >  	return siphash_3u32((__force u32)saddr, (__force u32)daddr,
> 
> Should you be doing the same with secure_ipv6_port_ephemeral() too? Why
> the asymmetry?

I remember not finding it in the similar code path, but maybe I missed
something. It's used by inet6_sk_port_offset() which also returns a u32,
itself used by inet6_hash_connect() and passed to __inet_hash_connect().

Hmmm the loop is now closed, I don't know how I missed it. So yes I
agree that it would definitely be needed. I'll update the patch, many
thanks!

Willy

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

* Re: [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash
  2022-04-27  6:52 ` [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash Willy Tarreau
  2022-04-27  9:56   ` kernel test robot
  2022-04-27 17:18   ` Jason A. Donenfeld
@ 2022-04-28  1:59   ` kernel test robot
  2 siblings, 0 replies; 24+ messages in thread
From: kernel test robot @ 2022-04-28  1:59 UTC (permalink / raw)
  To: Willy Tarreau, netdev
  Cc: kbuild-all, Jakub Kicinski, Eric Dumazet, Moshe Kol, Yossi Gilad,
	Amit Klein, linux-kernel, Willy Tarreau, Jason A . Donenfeld

Hi Willy,

I love your patch! Yet something to improve:

[auto build test ERROR on net/master]

url:    https://github.com/intel-lab-lkp/linux/commits/Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git 71cffebf6358a7f5031f5b208bbdc1cb4db6e539
config: i386-randconfig-a011-20220425 (https://download.01.org/0day-ci/archive/20220428/202204280348.UBtfnU6Q-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.2.0-20) 11.2.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/01b26e522b598adf346b809075880feab3dcdc08
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Willy-Tarreau/insufficient-TCP-source-port-randomness/20220427-145651
        git checkout 01b26e522b598adf346b809075880feab3dcdc08
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: net/ipv4/inet_hashtables.o: in function `__inet_hash_connect':
>> net/ipv4/inet_hashtables.c:780: undefined reference to `__umoddi3'


vim +780 net/ipv4/inet_hashtables.c

190cc82489f46f Eric Dumazet             2021-02-09  735  
5ee31fc1ecdcbc Pavel Emelyanov          2008-01-31  736  int __inet_hash_connect(struct inet_timewait_death_row *death_row,
01b26e522b598a Willy Tarreau            2022-04-27  737  		struct sock *sk, u64 port_offset,
5ee31fc1ecdcbc Pavel Emelyanov          2008-01-31  738  		int (*check_established)(struct inet_timewait_death_row *,
b4d6444ea3b50b Eric Dumazet             2015-03-18  739  			struct sock *, __u16, struct inet_timewait_sock **))
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  740  {
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  741  	struct inet_hashinfo *hinfo = death_row->hashinfo;
1580ab63fc9a03 Eric Dumazet             2016-02-11  742  	struct inet_timewait_sock *tw = NULL;
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  743  	struct inet_bind_hashbucket *head;
1580ab63fc9a03 Eric Dumazet             2016-02-11  744  	int port = inet_sk(sk)->inet_num;
3b1e0a655f8eba YOSHIFUJI Hideaki        2008-03-26  745  	struct net *net = sock_net(sk);
1580ab63fc9a03 Eric Dumazet             2016-02-11  746  	struct inet_bind_bucket *tb;
1580ab63fc9a03 Eric Dumazet             2016-02-11  747  	u32 remaining, offset;
1580ab63fc9a03 Eric Dumazet             2016-02-11  748  	int ret, i, low, high;
3c82a21f4320c8 Robert Shearman          2018-11-07  749  	int l3mdev;
190cc82489f46f Eric Dumazet             2021-02-09  750  	u32 index;
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  751  
1580ab63fc9a03 Eric Dumazet             2016-02-11  752  	if (port) {
1580ab63fc9a03 Eric Dumazet             2016-02-11  753  		head = &hinfo->bhash[inet_bhashfn(net, port,
1580ab63fc9a03 Eric Dumazet             2016-02-11  754  						  hinfo->bhash_size)];
1580ab63fc9a03 Eric Dumazet             2016-02-11  755  		tb = inet_csk(sk)->icsk_bind_hash;
1580ab63fc9a03 Eric Dumazet             2016-02-11  756  		spin_lock_bh(&head->lock);
1580ab63fc9a03 Eric Dumazet             2016-02-11  757  		if (sk_head(&tb->owners) == sk && !sk->sk_bind_node.next) {
01770a16616573 Ricardo Dias             2020-11-20  758  			inet_ehash_nolisten(sk, NULL, NULL);
1580ab63fc9a03 Eric Dumazet             2016-02-11  759  			spin_unlock_bh(&head->lock);
1580ab63fc9a03 Eric Dumazet             2016-02-11  760  			return 0;
1580ab63fc9a03 Eric Dumazet             2016-02-11  761  		}
1580ab63fc9a03 Eric Dumazet             2016-02-11  762  		spin_unlock(&head->lock);
1580ab63fc9a03 Eric Dumazet             2016-02-11  763  		/* No definite answer... Walk to established hash table */
1580ab63fc9a03 Eric Dumazet             2016-02-11  764  		ret = check_established(death_row, sk, port, NULL);
1580ab63fc9a03 Eric Dumazet             2016-02-11  765  		local_bh_enable();
1580ab63fc9a03 Eric Dumazet             2016-02-11  766  		return ret;
1580ab63fc9a03 Eric Dumazet             2016-02-11  767  	}
227b60f5102cda Stephen Hemminger        2007-10-10  768  
3c82a21f4320c8 Robert Shearman          2018-11-07  769  	l3mdev = inet_sk_bound_l3mdev(sk);
3c82a21f4320c8 Robert Shearman          2018-11-07  770  
1580ab63fc9a03 Eric Dumazet             2016-02-11  771  	inet_get_local_port_range(net, &low, &high);
1580ab63fc9a03 Eric Dumazet             2016-02-11  772  	high++; /* [32768, 60999] -> [32768, 61000[ */
1580ab63fc9a03 Eric Dumazet             2016-02-11  773  	remaining = high - low;
1580ab63fc9a03 Eric Dumazet             2016-02-11  774  	if (likely(remaining > 1))
1580ab63fc9a03 Eric Dumazet             2016-02-11  775  		remaining &= ~1U;
1580ab63fc9a03 Eric Dumazet             2016-02-11  776  
190cc82489f46f Eric Dumazet             2021-02-09  777  	net_get_random_once(table_perturb, sizeof(table_perturb));
190cc82489f46f Eric Dumazet             2021-02-09  778  	index = hash_32(port_offset, INET_TABLE_PERTURB_SHIFT);
190cc82489f46f Eric Dumazet             2021-02-09  779  
190cc82489f46f Eric Dumazet             2021-02-09 @780  	offset = (READ_ONCE(table_perturb[index]) + port_offset) % remaining;
1580ab63fc9a03 Eric Dumazet             2016-02-11  781  	/* In first pass we try ports of @low parity.
1580ab63fc9a03 Eric Dumazet             2016-02-11  782  	 * inet_csk_get_port() does the opposite choice.
07f4c90062f8fc Eric Dumazet             2015-05-24  783  	 */
1580ab63fc9a03 Eric Dumazet             2016-02-11  784  	offset &= ~1U;
1580ab63fc9a03 Eric Dumazet             2016-02-11  785  other_parity_scan:
1580ab63fc9a03 Eric Dumazet             2016-02-11  786  	port = low + offset;
1580ab63fc9a03 Eric Dumazet             2016-02-11  787  	for (i = 0; i < remaining; i += 2, port += 2) {
1580ab63fc9a03 Eric Dumazet             2016-02-11  788  		if (unlikely(port >= high))
1580ab63fc9a03 Eric Dumazet             2016-02-11  789  			port -= remaining;
122ff243f5f104 WANG Cong                2014-05-12  790  		if (inet_is_local_reserved_port(net, port))
e3826f1e946e7d Amerigo Wang             2010-05-05  791  			continue;
7f635ab71eef8d Pavel Emelyanov          2008-06-16  792  		head = &hinfo->bhash[inet_bhashfn(net, port,
7f635ab71eef8d Pavel Emelyanov          2008-06-16  793  						  hinfo->bhash_size)];
1580ab63fc9a03 Eric Dumazet             2016-02-11  794  		spin_lock_bh(&head->lock);
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  795  
1580ab63fc9a03 Eric Dumazet             2016-02-11  796  		/* Does not bother with rcv_saddr checks, because
1580ab63fc9a03 Eric Dumazet             2016-02-11  797  		 * the established check is already unique enough.
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  798  		 */
b67bfe0d42cac5 Sasha Levin              2013-02-27  799  		inet_bind_bucket_for_each(tb, &head->chain) {
3c82a21f4320c8 Robert Shearman          2018-11-07  800  			if (net_eq(ib_net(tb), net) && tb->l3mdev == l3mdev &&
3c82a21f4320c8 Robert Shearman          2018-11-07  801  			    tb->port == port) {
da5e36308d9f71 Tom Herbert              2013-01-22  802  				if (tb->fastreuse >= 0 ||
da5e36308d9f71 Tom Herbert              2013-01-22  803  				    tb->fastreuseport >= 0)
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  804  					goto next_port;
a9d8f9110d7e95 Evgeniy Polyakov         2009-01-19  805  				WARN_ON(hlist_empty(&tb->owners));
5ee31fc1ecdcbc Pavel Emelyanov          2008-01-31  806  				if (!check_established(death_row, sk,
5ee31fc1ecdcbc Pavel Emelyanov          2008-01-31  807  						       port, &tw))
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  808  					goto ok;
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  809  				goto next_port;
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  810  			}
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  811  		}
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  812  
941b1d22cc035a Pavel Emelyanov          2008-01-31  813  		tb = inet_bind_bucket_create(hinfo->bind_bucket_cachep,
3c82a21f4320c8 Robert Shearman          2018-11-07  814  					     net, head, port, l3mdev);
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  815  		if (!tb) {
1580ab63fc9a03 Eric Dumazet             2016-02-11  816  			spin_unlock_bh(&head->lock);
1580ab63fc9a03 Eric Dumazet             2016-02-11  817  			return -ENOMEM;
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  818  		}
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  819  		tb->fastreuse = -1;
da5e36308d9f71 Tom Herbert              2013-01-22  820  		tb->fastreuseport = -1;
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  821  		goto ok;
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  822  next_port:
1580ab63fc9a03 Eric Dumazet             2016-02-11  823  		spin_unlock_bh(&head->lock);
1580ab63fc9a03 Eric Dumazet             2016-02-11  824  		cond_resched();
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  825  	}
1580ab63fc9a03 Eric Dumazet             2016-02-11  826  
1580ab63fc9a03 Eric Dumazet             2016-02-11  827  	offset++;
1580ab63fc9a03 Eric Dumazet             2016-02-11  828  	if ((offset & 1) && remaining > 1)
1580ab63fc9a03 Eric Dumazet             2016-02-11  829  		goto other_parity_scan;
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  830  
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  831  	return -EADDRNOTAVAIL;
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  832  
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  833  ok:
c579bd1b4021c4 Eric Dumazet             2021-02-09  834  	/* If our first attempt found a candidate, skip next candidate
c579bd1b4021c4 Eric Dumazet             2021-02-09  835  	 * in 1/16 of cases to add some noise.
c579bd1b4021c4 Eric Dumazet             2021-02-09  836  	 */
c579bd1b4021c4 Eric Dumazet             2021-02-09  837  	if (!i && !(prandom_u32() % 16))
c579bd1b4021c4 Eric Dumazet             2021-02-09  838  		i = 2;
190cc82489f46f Eric Dumazet             2021-02-09  839  	WRITE_ONCE(table_perturb[index], READ_ONCE(table_perturb[index]) + i + 2);
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  840  
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  841  	/* Head lock still held and bh's disabled */
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  842  	inet_bind_hash(sk, tb, port);
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  843  	if (sk_unhashed(sk)) {
c720c7e8383aff Eric Dumazet             2009-10-15  844  		inet_sk(sk)->inet_sport = htons(port);
01770a16616573 Ricardo Dias             2020-11-20  845  		inet_ehash_nolisten(sk, (struct sock *)tw, NULL);
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  846  	}
3cdaedae635b17 Eric Dumazet             2009-12-04  847  	if (tw)
fc01538f9fb755 Eric Dumazet             2015-07-08  848  		inet_twsk_bind_unhash(tw, hinfo);
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  849  	spin_unlock(&head->lock);
dbe7faa4045ea8 Eric Dumazet             2015-07-08  850  	if (tw)
dbe7faa4045ea8 Eric Dumazet             2015-07-08  851  		inet_twsk_deschedule_put(tw);
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  852  	local_bh_enable();
1580ab63fc9a03 Eric Dumazet             2016-02-11  853  	return 0;
a7f5e7f164788a Arnaldo Carvalho de Melo 2005-12-13  854  }
5ee31fc1ecdcbc Pavel Emelyanov          2008-01-31  855  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

end of thread, other threads:[~2022-04-28  2:00 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-27  6:52 [PATCH net 0/7] insufficient TCP source port randomness Willy Tarreau
2022-04-27  6:52 ` [PATCH net 1/7] secure_seq: return the full 64-bit of the siphash Willy Tarreau
2022-04-27  9:56   ` kernel test robot
2022-04-27 10:07     ` Willy Tarreau
2022-04-27 10:07       ` Willy Tarreau
2022-04-27 16:35       ` Willy Tarreau
2022-04-27 16:35         ` Willy Tarreau
2022-04-27 16:50         ` Eric Dumazet
2022-04-27 16:50           ` Eric Dumazet
2022-04-27 16:56           ` Willy Tarreau
2022-04-27 16:56             ` Willy Tarreau
2022-04-27 17:18   ` Jason A. Donenfeld
2022-04-27 20:19     ` Willy Tarreau
2022-04-28  1:59   ` kernel test robot
2022-04-27  6:52 ` [PATCH net 2/7] tcp: use different parts of the port_offset for index and offset Willy Tarreau
2022-04-27  6:52 ` [PATCH net 3/7] tcp: resalt the secret every 10 seconds Willy Tarreau
2022-04-27 15:56   ` Stephen Hemminger
2022-04-27 16:21     ` Willy Tarreau
2022-04-27  6:52 ` [PATCH net 4/7] tcp: add small random increments to the source port Willy Tarreau
2022-04-27  6:52 ` [PATCH net 5/7] tcp: dynamically allocate the perturb table used by source ports Willy Tarreau
2022-04-27  6:52 ` [PATCH net 6/7] tcp: increase source port perturb table to 2^16 Willy Tarreau
2022-04-27  8:07   ` David Laight
2022-04-27  8:19     ` Willy Tarreau
2022-04-27  6:52 ` [PATCH net 7/7] tcp: drop the hash_32() part from the index calculation Willy Tarreau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.