Re: [PATCH net] udp_tunnel: drop packets when hibernating

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Jason A. Donenfeld wrote:
> The kernel's various networking applications keep churning away after
> userspace is frozen during hibernation, even as a memory snapshot is
> being made. This can lead many network applications to an inconsistent
> state, replaying packets and cryptographic state changes. For example,
> on wireguard, there's the possibility of this sequence:
> 
> 1) hibernating begins
> 2) handshake state cleared
> 3) keypairs cleared
> 4) new handshake round trip completes
> 5) machine memory is snapshotted
> 6) packet is sent using new keypair
> 7) machine is restored to state (5)
> 8) packet is sent using new keypair
> 
> The idea is to prevent (6) from happening, especially if (6) and (8)
> contain different data, but the same key and nonce. Presumably the same
> issue applies to other users of udp_tunnel too.
> 
> Fix this by just dropping sending and receiving packets during the
> hibernation sequence.

A few high level questions:

If the issue is reuse of key + nonce during send, why include receive
side functions? Specifically tunnel (encap_rcv) functions.

Is this a problem specific to UDP tunnels?
 
> Cc: stable@vger.kernel.org
> Reported-by: Jérémy Jean <jeremy.jean@oss.cyber.gouv.fr>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> I wrote this patch in response to the issue Jérémy raised, but I'm not
> actually super familiar with all of the hibernation mechanics. If
> somebody working on PM would think about this matter too, I'd be much
> obliged.
> 
>  include/linux/freezer.h    | 1 +
>  net/ipv4/udp.c             | 5 +++++
>  net/ipv4/udp_tunnel_core.c | 5 +++++
>  net/ipv6/ip6_udp_tunnel.c  | 5 +++++
>  net/ipv6/udp.c             | 5 +++++
>  5 files changed, 21 insertions(+)
> 
> diff --git a/include/linux/freezer.h b/include/linux/freezer.h
> index 0a8c6c4d1a82..21d708dc4092 100644
> --- a/include/linux/freezer.h
> +++ b/include/linux/freezer.h
> @@ -76,6 +76,7 @@ static inline bool cgroup1_freezing(struct task_struct *task)
>  #endif /* !CONFIG_CGROUP_FREEZER */
>  
>  #else /* !CONFIG_FREEZER */
> +#define pm_freezing (false)
>  static inline bool frozen(struct task_struct *p) { return false; }
>  static inline bool freezing(struct task_struct *p) { return false; }
>  static inline void __thaw_task(struct task_struct *t) {}
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index b090bd1f59e8..5021932ad9f1 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -95,6 +95,7 @@
>  #include <linux/netdevice.h>
>  #include <linux/slab.h>
>  #include <linux/sock_diag.h>
> +#include <linux/freezer.h>
>  #include <net/tcp_states.h>
>  #include <linux/skbuff.h>
>  #include <linux/proc_fs.h>
> @@ -2425,6 +2426,10 @@ static int udp_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
>  		if (encap_rcv) {
>  			int ret;
>  
> +			/* Drop if we're hibernating */
> +			if (unlikely(pm_freezing))
> +				goto drop;
> +
>  			/* Verify checksum before giving to encap */
>  			if (udp_lib_checksum_complete(skb))
>  				goto csum_error;
> diff --git a/net/ipv4/udp_tunnel_core.c b/net/ipv4/udp_tunnel_core.c
> index a128fe85620d..e3666ed96af7 100644
> --- a/net/ipv4/udp_tunnel_core.c
> +++ b/net/ipv4/udp_tunnel_core.c
> @@ -3,6 +3,7 @@
>  #include <linux/errno.h>
>  #include <linux/socket.h>
>  #include <linux/kernel.h>
> +#include <linux/freezer.h>
>  #include <net/dst_metadata.h>
>  #include <net/flow.h>
>  #include <net/udp.h>
> @@ -172,6 +173,10 @@ void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb
>  {
>  	struct udphdr *uh;
>  
> +	/* Drop if we're hibernating */
> +	if (unlikely(pm_freezing))
> +		return;
> +
>  	__skb_push(skb, sizeof(*uh));
>  	skb_reset_transport_header(skb);
>  	uh = udp_hdr(skb);
> diff --git a/net/ipv6/ip6_udp_tunnel.c b/net/ipv6/ip6_udp_tunnel.c
> index 32525a051a6f..4a31e8cc8887 100644
> --- a/net/ipv6/ip6_udp_tunnel.c
> +++ b/net/ipv6/ip6_udp_tunnel.c
> @@ -7,6 +7,7 @@
>  #include <linux/types.h>
>  #include <linux/kernel.h>
>  #include <linux/in6.h>
> +#include <linux/freezer.h>
>  #include <net/udp.h>
>  #include <net/udp_tunnel.h>
>  #include <net/net_namespace.h>
> @@ -86,6 +87,10 @@ void udp_tunnel6_xmit_skb(struct dst_entry *dst, struct sock *sk,
>  	struct udphdr *uh;
>  	struct ipv6hdr *ip6h;
>  
> +	/* Drop if we're hibernating */
> +	if (unlikely(pm_freezing))
> +		return;
> +
>  	__skb_push(skb, sizeof(*uh));
>  	skb_reset_transport_header(skb);
>  	uh = udp_hdr(skb);
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 93478d1ad576..db9c2050887d 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -34,6 +34,7 @@
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
>  #include <linux/indirect_call_wrapper.h>
> +#include <linux/freezer.h>
>  #include <trace/events/udp.h>
>  
>  #include <net/addrconf.h>
> @@ -848,6 +849,10 @@ static int udpv6_queue_rcv_one_skb(struct sock *sk, struct sk_buff *skb)
>  		if (encap_rcv) {
>  			int ret;
>  
> +			/* Drop if we're hibernating */
> +			if (unlikely(pm_freezing))
> +				goto drop;
> +
>  			/* Verify checksum before giving to encap */
>  			if (udp_lib_checksum_complete(skb))
>  				goto csum_error;
> -- 
> 2.56.0
>