[net] udp: drop skb extensions before marking skb stateless

Once udp stack has set the UDP_SKB_IS_STATELESS flag, later skb free
assumes all skb head state has been dropped already.

This will leak the extension memory in case the skb has extensions other
than the ipsec secpath, e.g. bridge nf data.

To fix this, set the UDP_SKB_IS_STATELESS flag only if we don't have
extensions or if the extension space can be free'd.

Fixes: 895b5c9f206eb7d25dc1360a ("netfilter: drop bridge nf reset from nf_reset")
Cc: Paolo Abeni <pabeni@redhat.com>
Reported-by: Byron Stanoszek <gandalf@winds.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/linux/skbuff.h |  6 ++++++
 net/ipv4/udp.c         | 27 ++++++++++++++++++++++-----
 2 files changed, 28 insertions(+), 5 deletions(-)

On Thu, 2019-11-21 at 06:56 +0100, Florian Westphal wrote:
> Once udp stack has set the UDP_SKB_IS_STATELESS flag, later skb free
> assumes all skb head state has been dropped already.
> 
> This will leak the extension memory in case the skb has extensions other
> than the ipsec secpath, e.g. bridge nf data.
> 
> To fix this, set the UDP_SKB_IS_STATELESS flag only if we don't have
> extensions or if the extension space can be free'd.
> 
> Fixes: 895b5c9f206eb7d25dc1360a ("netfilter: drop bridge nf reset from nf_reset")
> Cc: Paolo Abeni <pabeni@redhat.com>
> Reported-by: Byron Stanoszek <gandalf@winds.org>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  include/linux/skbuff.h |  6 ++++++
>  net/ipv4/udp.c         | 27 ++++++++++++++++++++++-----
>  2 files changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 64a395c7f689..8688f7adfda7 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -4169,12 +4169,18 @@ static inline void skb_ext_reset(struct sk_buff *skb)
>  		skb->active_extensions = 0;
>  	}
>  }
> +
> +static inline bool skb_has_extensions(struct sk_buff *skb)
> +{
> +	return unlikely(skb->active_extensions);
> +}
>  #else
>  static inline void skb_ext_put(struct sk_buff *skb) {}
>  static inline void skb_ext_reset(struct sk_buff *skb) {}
>  static inline void skb_ext_del(struct sk_buff *skb, int unused) {}
>  static inline void __skb_ext_copy(struct sk_buff *d, const struct sk_buff *s) {}
>  static inline void skb_ext_copy(struct sk_buff *dst, const struct sk_buff *s) {}
> +static inline bool skb_has_extensions(struct sk_buff *skb) { return false; }
>  #endif /* CONFIG_SKB_EXTENSIONS */
>  
>  static inline void nf_reset_ct(struct sk_buff *skb)
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 1d58ce829dca..447defbfccdd 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1297,6 +1297,27 @@ int udp_sendpage(struct sock *sk, struct page *page, int offset,
>  
>  #define UDP_SKB_IS_STATELESS 0x80000000
>  
> +/* all head states (dst, sk, nf conntrack) except skb extensions are
> + * cleared by udp_rcv().
> + *
> + * We need to preserve secpath, if present, to eventually process
> + * IP_CMSG_PASSSEC at recvmsg() time.
> + *
> + * Other extensions can be cleared.
> + */
> +static bool udp_try_make_stateless(struct sk_buff *skb)
> +{
> +	if (!skb_has_extensions(skb))
> +		return true;
> +
> +	if (!secpath_exists(skb)) {
> +		skb_ext_reset(skb);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
>  static void udp_set_dev_scratch(struct sk_buff *skb)
>  {
>  	struct udp_dev_scratch *scratch = udp_skb_scratch(skb);
> @@ -1308,11 +1329,7 @@ static void udp_set_dev_scratch(struct sk_buff *skb)
>  	scratch->csum_unnecessary = !!skb_csum_unnecessary(skb);
>  	scratch->is_linear = !skb_is_nonlinear(skb);
>  #endif
> -	/* all head states execept sp (dst, sk, nf) are always cleared by
> -	 * udp_rcv() and we need to preserve secpath, if present, to eventually
> -	 * process IP_CMSG_PASSSEC at recvmsg() time
> -	 */
> -	if (likely(!skb_sec_path(skb)))
> +	if (udp_try_make_stateless(skb))
>  		scratch->_tsize_state |= UDP_SKB_IS_STATELESS;
>  }

Acked-by: Paolo Abeni <pabeni@redhat.com>

On Thu, 21 Nov 2019, Florian Westphal wrote:

> Once udp stack has set the UDP_SKB_IS_STATELESS flag, later skb free
> assumes all skb head state has been dropped already.
>
> This will leak the extension memory in case the skb has extensions other
> than the ipsec secpath, e.g. bridge nf data.
>
> To fix this, set the UDP_SKB_IS_STATELESS flag only if we don't have
> extensions or if the extension space can be free'd.
>
> Fixes: 895b5c9f206eb7d25dc1360a ("netfilter: drop bridge nf reset from nf_reset")
> Cc: Paolo Abeni <pabeni@redhat.com>
> Reported-by: Byron Stanoszek <gandalf@winds.org>
> Signed-off-by: Florian Westphal <fw@strlen.de>

I confirm that this fixes the memory leak on my systems. Thank you for the fast
turnaround.

Regards,
  -Byron

From: Florian Westphal <fw@strlen.de>
Date: Thu, 21 Nov 2019 06:56:23 +0100

> Once udp stack has set the UDP_SKB_IS_STATELESS flag, later skb free
> assumes all skb head state has been dropped already.
> 
> This will leak the extension memory in case the skb has extensions other
> than the ipsec secpath, e.g. bridge nf data.
> 
> To fix this, set the UDP_SKB_IS_STATELESS flag only if we don't have
> extensions or if the extension space can be free'd.
> 
> Fixes: 895b5c9f206eb7d25dc1360a ("netfilter: drop bridge nf reset from nf_reset")
> Cc: Paolo Abeni <pabeni@redhat.com>
> Reported-by: Byron Stanoszek <gandalf@winds.org>
> Signed-off-by: Florian Westphal <fw@strlen.de>

Applied, thanks Florian.