netdev - Re: [PATCH v2 07/17] net: tcp: allow tcp_timewait_sock to validate skbs before handing to device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <685c8dff7491f_2a5da429443@willemb.c.googlers.com.notmuch>
Date: Wed, 25 Jun 2025 20:02:07 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Daniel Zahka <daniel.zahka@...il.com>, 
 Donald Hunter <donald.hunter@...il.com>, 
 Jakub Kicinski <kuba@...nel.org>, 
 "David S. Miller" <davem@...emloft.net>, 
 Eric Dumazet <edumazet@...gle.com>, 
 Paolo Abeni <pabeni@...hat.com>, 
 Simon Horman <horms@...nel.org>, 
 Jonathan Corbet <corbet@....net>, 
 Andrew Lunn <andrew+netdev@...n.ch>
Cc: Saeed Mahameed <saeedm@...dia.com>, 
 Leon Romanovsky <leon@...nel.org>, 
 Tariq Toukan <tariqt@...dia.com>, 
 Boris Pismenny <borisp@...dia.com>, 
 Kuniyuki Iwashima <kuniyu@...gle.com>, 
 Willem de Bruijn <willemb@...gle.com>, 
 David Ahern <dsahern@...nel.org>, 
 Neal Cardwell <ncardwell@...gle.com>, 
 Patrisious Haddad <phaddad@...dia.com>, 
 Raed Salem <raeds@...dia.com>, 
 Jianbo Liu <jianbol@...dia.com>, 
 Dragos Tatulea <dtatulea@...dia.com>, 
 Rahul Rameshbabu <rrameshbabu@...dia.com>, 
 Stanislav Fomichev <sdf@...ichev.me>, 
 Toke Høiland-Jørgensen <toke@...hat.com>, 
 Alexander Lobakin <aleksander.lobakin@...el.com>, 
 Jacob Keller <jacob.e.keller@...el.com>, 
 netdev@...r.kernel.org
Subject: Re: [PATCH v2 07/17] net: tcp: allow tcp_timewait_sock to validate
 skbs before handing to device

Daniel Zahka wrote:
> Provide a callback to validate skb's originating from tcp timewait
> socks before passing to the device layer. Full socks have a
> sk_validate_xmit_skb member for checking that a device is capable of
> performing offloads required for transmitting an skb. With psp, tcp
> timewait socks will inherit the crypto state from their corresponding
> full socks. Any ACKs or RSTs that originate from a tcp timewait sock
> carrying psp state should be psp encapsulated.
> 
> Signed-off-by: Daniel Zahka <daniel.zahka@...il.com>
> ---
> 
> Notes:
>     v2:
>     - patch introduced in v2
> 
>  include/net/inet_timewait_sock.h |  5 +++++
>  net/core/dev.c                   | 14 ++++++++++++--
>  net/ipv4/inet_timewait_sock.c    |  3 +++
>  3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/inet_timewait_sock.h b/include/net/inet_timewait_sock.h
> index c1295246216c..3a31c74c9e15 100644
> --- a/include/net/inet_timewait_sock.h
> +++ b/include/net/inet_timewait_sock.h
> @@ -84,6 +84,11 @@ struct inet_timewait_sock {
>  #if IS_ENABLED(CONFIG_INET_PSP)
>  	struct psp_assoc __rcu	  *psp_assoc;
>  #endif
> +#ifdef CONFIG_SOCK_VALIDATE_XMIT
> +	struct sk_buff*		(*tw_validate_xmit_skb)(struct sock *sk,
> +							struct net_device *dev,
> +							struct sk_buff *skb);
> +#endif
>  };
>  #define tw_tclass tw_tos
>  
> diff --git a/net/core/dev.c b/net/core/dev.c
> index b825b3f5b7db..bf013436a57b 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3904,10 +3904,20 @@ static struct sk_buff *sk_validate_xmit_skb(struct sk_buff *skb,
>  					    struct net_device *dev)
>  {
>  #ifdef CONFIG_SOCK_VALIDATE_XMIT
> +	struct sk_buff *(*sk_validate)(struct sock *sk, struct net_device *dev,
> +				       struct sk_buff *skb);
>  	struct sock *sk = skb->sk;
>  
> -	if (sk && sk_fullsock(sk) && sk->sk_validate_xmit_skb) {
> -		skb = sk->sk_validate_xmit_skb(sk, dev, skb);
> +	sk_validate = NULL;
> +	if (sk) {
> +		if (sk_fullsock(sk))
> +			sk_validate = sk->sk_validate_xmit_skb;
> +		else if (sk->sk_state == TCP_TIME_WAIT)

Can this shadow a different constant for other protocols?

To be on the safe side: sk_is_tcp

> +			sk_validate = inet_twsk(sk)->tw_validate_xmit_skb;
> +	}
> +
> +	if (sk_validate) {
> +		skb = sk_validate(sk, dev, skb);

If this callback is in the hot path, a candidate for an INDIRECT_CALL wrapper.
There are only two users, TLS and PSP.

>  	} else if (unlikely(skb_is_decrypted(skb))) {
>  		pr_warn_ratelimited("unencrypted skb with no associated socket - dropping\n");
>  		kfree_skb(skb);
> diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
> index dfde7895d8f2..859c03e07466 100644
> --- a/net/ipv4/inet_timewait_sock.c
> +++ b/net/ipv4/inet_timewait_sock.c
> @@ -210,6 +210,9 @@ struct inet_timewait_sock *inet_twsk_alloc(const struct sock *sk,
>  		atomic64_set(&tw->tw_cookie, atomic64_read(&sk->sk_cookie));
>  		twsk_net_set(tw, sock_net(sk));
>  		timer_setup(&tw->tw_timer, tw_timer_handler, 0);
> +#ifdef CONFIG_SOCK_VALIDATE_XMIT
> +		tw->tw_validate_xmit_skb = NULL;
> +#endif
>  		/*
>  		 * Because we use RCU lookups, we should not set tw_refcnt
>  		 * to a non null value before everything is setup for this
> -- 
> 2.47.1
>