public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 net-next] tcp: Initialise ehash secrets during connect() and listen().
@ 2026-03-02 20:51 Kuniyuki Iwashima
  2026-03-03  3:42 ` Eric Dumazet
  0 siblings, 1 reply; 3+ messages in thread
From: Kuniyuki Iwashima @ 2026-03-02 20:51 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Neal Cardwell
  Cc: Simon Horman, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

inet_ehashfn() and inet6_ehashfn() initialise random secrets
on the first call by net_get_random_once().

While the init part is patched out using static keys, with
CONFIG_STACKPROTECTOR_STRONG=y, this causes a compiler to
generate a stack canary due to an automatic variable,
unsigned long ___flags, in the DO_ONCE() macro being passed
to __do_once_start().

With FDO, this is visible in __inet_lookup_established() and
__inet6_lookup_established() too.

Let's move net_get_random_once() to the slow paths: inet_hash()
for listen(), and inet_hash_connect() and inet6_hash_connect()
for connect().

Note that IPv6 listener will initialise both IPv4 & IPv6 secrets
in inet_hash() for IPv4-mapped IPv6 address.

With the patch, the stack size is reduced by 16 bytes (___flags
 + a stack canary) and NOPs for the static key go away.

Before: __inet6_lookup_established()

       ...
       push   %rbx
       sub    $0x38,%rsp                # stack is 56 bytes
       mov    %edx,%ebx                 # sport
       mov    %gs:0x299419f(%rip),%rax  # load stack canary
       mov    %rax,0x30(%rsp)              and store it onto stack
       mov    0x440(%rdi),%r15          # net->ipv4.tcp_death_row.hashinfo
       nop
 32:   mov    %r8d,%ebp                 # hnum
       shl    $0x10,%ebp                # hnum << 16
       nop
 3d:   mov    0x70(%rsp),%r14d          # sdif
       or     %ebx,%ebp                 # INET_COMBINED_PORTS(sport, hnum)
       mov    0x11a8382(%rip),%eax      # inet6_ehashfn() ...

After: __inet6_lookup_established()

       ...
       push   %rbx
       sub    $0x28,%rsp                # stack is 40 bytes
       mov    0x60(%rsp),%ebp           # sdif
       mov    %r8d,%r14d                # hnum
       shl    $0x10,%r14d               # hnum << 16
       or     %edx,%r14d                # INET_COMBINED_PORTS(sport, hnum)
       mov    0x440(%rdi),%rax          # net->ipv4.tcp_death_row.hashinfo
       mov    0x1194f09(%rip),%r10d     # inet6_ehashfn() ...

Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 include/net/inet6_hashtables.h |  2 ++
 net/ipv4/inet_hashtables.c     | 16 ++++++++++++++--
 net/ipv6/inet6_hashtables.c    | 11 ++++++++---
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/include/net/inet6_hashtables.h b/include/net/inet6_hashtables.h
index 282e29237d93..c17e0d874808 100644
--- a/include/net/inet6_hashtables.h
+++ b/include/net/inet6_hashtables.h
@@ -24,6 +24,8 @@
 
 struct inet_hashinfo;
 
+void inet6_init_ehash_secret(void);
+
 static inline unsigned int __inet6_ehashfn(const u32 lhash,
 				    const u16 lport,
 				    const u32 fhash,
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index fca980772c81..2c2cac5b0f1a 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -30,12 +30,15 @@
 #include <net/sock_reuseport.h>
 #include <net/tcp.h>
 
+static void inet_init_ehash_secret(void)
+{
+	net_get_random_once(&inet_ehash_secret, sizeof(inet_ehash_secret));
+}
+
 u32 inet_ehashfn(const struct net *net, const __be32 laddr,
 		 const __u16 lport, const __be32 faddr,
 		 const __be16 fport)
 {
-	net_get_random_once(&inet_ehash_secret, sizeof(inet_ehash_secret));
-
 	return lport + __inet_ehashfn(laddr, 0, faddr, fport,
 				      inet_ehash_secret + net_hash_mix(net));
 }
@@ -793,6 +796,13 @@ int inet_hash(struct sock *sk)
 		local_bh_enable();
 		return 0;
 	}
+
+#if IS_ENABLED(CONFIG_IPV6)
+	if (sk->sk_family == AF_INET6)
+		inet6_init_ehash_secret();
+#endif
+	inet_init_ehash_secret();
+
 	WARN_ON(!sk_unhashed(sk));
 	ilb2 = inet_lhash2_bucket_sk(hashinfo, sk);
 
@@ -1239,6 +1249,8 @@ int inet_hash_connect(struct inet_timewait_death_row *death_row,
 	if (!inet_sk(sk)->inet_num)
 		port_offset = inet_sk_port_offset(sk);
 
+	inet_init_ehash_secret();
+
 	hash_port0 = inet_ehashfn(net, inet->inet_rcv_saddr, 0,
 				  inet->inet_daddr, inet->inet_dport);
 
diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
index 5e1da088d8e1..054f75d32381 100644
--- a/net/ipv6/inet6_hashtables.c
+++ b/net/ipv6/inet6_hashtables.c
@@ -23,15 +23,18 @@
 #include <net/sock_reuseport.h>
 #include <net/tcp.h>
 
+void inet6_init_ehash_secret(void)
+{
+	net_get_random_once(&inet6_ehash_secret, sizeof(inet6_ehash_secret));
+	net_get_random_once(&tcp_ipv6_hash_secret, sizeof(tcp_ipv6_hash_secret));
+}
+
 u32 inet6_ehashfn(const struct net *net,
 		  const struct in6_addr *laddr, const u16 lport,
 		  const struct in6_addr *faddr, const __be16 fport)
 {
 	u32 lhash, fhash;
 
-	net_get_random_once(&inet6_ehash_secret, sizeof(inet6_ehash_secret));
-	net_get_random_once(&tcp_ipv6_hash_secret, sizeof(tcp_ipv6_hash_secret));
-
 	lhash = (__force u32)laddr->s6_addr32[3];
 	fhash = __ipv6_addr_jhash(faddr, tcp_ipv6_hash_secret);
 
@@ -362,6 +365,8 @@ int inet6_hash_connect(struct inet_timewait_death_row *death_row,
 	if (!inet_sk(sk)->inet_num)
 		port_offset = inet6_sk_port_offset(sk);
 
+	inet6_init_ehash_secret();
+
 	hash_port0 = inet6_ehashfn(net, daddr, 0, saddr, inet->inet_dport);
 
 	return __inet_hash_connect(death_row, sk, port_offset, hash_port0,
-- 
2.53.0.473.g4a7958ca14-goog


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

* Re: [PATCH v1 net-next] tcp: Initialise ehash secrets during connect() and listen().
  2026-03-02 20:51 [PATCH v1 net-next] tcp: Initialise ehash secrets during connect() and listen() Kuniyuki Iwashima
@ 2026-03-03  3:42 ` Eric Dumazet
  2026-03-03  4:58   ` Kuniyuki Iwashima
  0 siblings, 1 reply; 3+ messages in thread
From: Eric Dumazet @ 2026-03-03  3:42 UTC (permalink / raw)
  To: Kuniyuki Iwashima
  Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Neal Cardwell,
	Simon Horman, Kuniyuki Iwashima, netdev

On Mon, Mar 2, 2026 at 9:51 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> inet_ehashfn() and inet6_ehashfn() initialise random secrets
> on the first call by net_get_random_once().
>
> While the init part is patched out using static keys, with
> CONFIG_STACKPROTECTOR_STRONG=y, this causes a compiler to
> generate a stack canary due to an automatic variable,
> unsigned long ___flags, in the DO_ONCE() macro being passed
> to __do_once_start().
>
> With FDO, this is visible in __inet_lookup_established() and
> __inet6_lookup_established() too.
>
> Let's move net_get_random_once() to the slow paths: inet_hash()
> for listen(), and inet_hash_connect() and inet6_hash_connect()
> for connect().
>
> Note that IPv6 listener will initialise both IPv4 & IPv6 secrets
> in inet_hash() for IPv4-mapped IPv6 address.
>
> With the patch, the stack size is reduced by 16 bytes (___flags
>  + a stack canary) and NOPs for the static key go away.
>
> Before: __inet6_lookup_established()
>
>
>
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
>  include/net/inet6_hashtables.h |  2 ++
>  net/ipv4/inet_hashtables.c     | 16 ++++++++++++++--
>  net/ipv6/inet6_hashtables.c    | 11 ++++++++---
>  3 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/inet6_hashtables.h b/include/net/inet6_hashtables.h
> index 282e29237d93..c17e0d874808 100644
> --- a/include/net/inet6_hashtables.h
> +++ b/include/net/inet6_hashtables.h
> @@ -24,6 +24,8 @@
>
>  struct inet_hashinfo;
>
> +void inet6_init_ehash_secret(void);
> +
>  static inline unsigned int __inet6_ehashfn(const u32 lhash,
>                                     const u16 lport,
>                                     const u32 fhash,
> diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
> index fca980772c81..2c2cac5b0f1a 100644
> --- a/net/ipv4/inet_hashtables.c
> +++ b/net/ipv4/inet_hashtables.c
> @@ -30,12 +30,15 @@
>  #include <net/sock_reuseport.h>
>  #include <net/tcp.h>
>
> +static void inet_init_ehash_secret(void)
> +{
> +       net_get_random_once(&inet_ehash_secret, sizeof(inet_ehash_secret));
> +}

Could this use get_random_sleepable_once()?
We are running in process context here.
Same remark for  inet6_init_ehash_secret()

Thanks.

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

* Re: [PATCH v1 net-next] tcp: Initialise ehash secrets during connect() and listen().
  2026-03-03  3:42 ` Eric Dumazet
@ 2026-03-03  4:58   ` Kuniyuki Iwashima
  0 siblings, 0 replies; 3+ messages in thread
From: Kuniyuki Iwashima @ 2026-03-03  4:58 UTC (permalink / raw)
  To: edumazet; +Cc: davem, horms, kuba, kuni1840, kuniyu, ncardwell, netdev, pabeni

From: Eric Dumazet <edumazet@google.com>
Date: Tue, 3 Mar 2026 04:42:24 +0100
> On Mon, Mar 2, 2026 at 9:51 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> >
> > inet_ehashfn() and inet6_ehashfn() initialise random secrets
> > on the first call by net_get_random_once().
> >
> > While the init part is patched out using static keys, with
> > CONFIG_STACKPROTECTOR_STRONG=y, this causes a compiler to
> > generate a stack canary due to an automatic variable,
> > unsigned long ___flags, in the DO_ONCE() macro being passed
> > to __do_once_start().
> >
> > With FDO, this is visible in __inet_lookup_established() and
> > __inet6_lookup_established() too.
> >
> > Let's move net_get_random_once() to the slow paths: inet_hash()
> > for listen(), and inet_hash_connect() and inet6_hash_connect()
> > for connect().
> >
> > Note that IPv6 listener will initialise both IPv4 & IPv6 secrets
> > in inet_hash() for IPv4-mapped IPv6 address.
> >
> > With the patch, the stack size is reduced by 16 bytes (___flags
> >  + a stack canary) and NOPs for the static key go away.
> >
> > Before: __inet6_lookup_established()
> >
> >
> >
> > Suggested-by: Eric Dumazet <edumazet@google.com>
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > ---
> >  include/net/inet6_hashtables.h |  2 ++
> >  net/ipv4/inet_hashtables.c     | 16 ++++++++++++++--
> >  net/ipv6/inet6_hashtables.c    | 11 ++++++++---
> >  3 files changed, 24 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/net/inet6_hashtables.h b/include/net/inet6_hashtables.h
> > index 282e29237d93..c17e0d874808 100644
> > --- a/include/net/inet6_hashtables.h
> > +++ b/include/net/inet6_hashtables.h
> > @@ -24,6 +24,8 @@
> >
> >  struct inet_hashinfo;
> >
> > +void inet6_init_ehash_secret(void);
> > +
> >  static inline unsigned int __inet6_ehashfn(const u32 lhash,
> >                                     const u16 lport,
> >                                     const u32 fhash,
> > diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
> > index fca980772c81..2c2cac5b0f1a 100644
> > --- a/net/ipv4/inet_hashtables.c
> > +++ b/net/ipv4/inet_hashtables.c
> > @@ -30,12 +30,15 @@
> >  #include <net/sock_reuseport.h>
> >  #include <net/tcp.h>
> >
> > +static void inet_init_ehash_secret(void)
> > +{
> > +       net_get_random_once(&inet_ehash_secret, sizeof(inet_ehash_secret));
> > +}
> 
> Could this use get_random_sleepable_once()?
> We are running in process context here.
> Same remark for  inet6_init_ehash_secret()

Ah that's better !

I'll squash this to v2, thanks !

---8<---
diff --git a/include/linux/net.h b/include/linux/net.h
index f58b38ab37f8..a8e818de95b3 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -304,6 +304,8 @@ do {									\
 
 #define net_get_random_once(buf, nbytes)			\
 	get_random_once((buf), (nbytes))
+#define net_get_random_sleepable_once(buf, nbytes)		\
+	get_random_sleepable_once((buf), (nbytes))
 
 /*
  * E.g. XFS meta- & log-data is in slab pages, or bcache meta
diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 2c2cac5b0f1a..e4c9a1590a74 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -32,7 +32,8 @@
 
 static void inet_init_ehash_secret(void)
 {
-	net_get_random_once(&inet_ehash_secret, sizeof(inet_ehash_secret));
+	net_get_random_sleepable_once(&inet_ehash_secret,
+				      sizeof(inet_ehash_secret));
 }
 
 u32 inet_ehashfn(const struct net *net, const __be32 laddr,
diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c
index 054f75d32381..a0e5a8812614 100644
--- a/net/ipv6/inet6_hashtables.c
+++ b/net/ipv6/inet6_hashtables.c
@@ -25,8 +25,10 @@
 
 void inet6_init_ehash_secret(void)
 {
-	net_get_random_once(&inet6_ehash_secret, sizeof(inet6_ehash_secret));
-	net_get_random_once(&tcp_ipv6_hash_secret, sizeof(tcp_ipv6_hash_secret));
+	net_get_random_sleepable_once(&inet6_ehash_secret,
+				      sizeof(inet6_ehash_secret));
+	net_get_random_sleepable_once(&tcp_ipv6_hash_secret,
+				      sizeof(tcp_ipv6_hash_secret));
 }
 
 u32 inet6_ehashfn(const struct net *net,
---8<---

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

end of thread, other threads:[~2026-03-03  4:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-02 20:51 [PATCH v1 net-next] tcp: Initialise ehash secrets during connect() and listen() Kuniyuki Iwashima
2026-03-03  3:42 ` Eric Dumazet
2026-03-03  4:58   ` Kuniyuki Iwashima

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox