[nf,v2] ipvs: fix dependency on nf_defrag_ipv6

Message ID 59710d42fdb1142ed1fc7fced89dee737200a333.1549897875.git.aclaudi@redhat.com
State Accepted
Delegated to: Pablo Neira
Headers show
Series
  • [nf,v2] ipvs: fix dependency on nf_defrag_ipv6
Related show

Commit Message

Andrea Claudi Feb. 11, 2019, 3:14 p.m.
ipvs relies on nf_defrag_ipv6 module to manage IPv6 fragmentation,
but lacks proper Kconfig dependencies and does not explicitly
request defrag features.

As a result, if netfilter hooks are not loaded, when IPv6 fragmented
packet are handled by ipvs only the first fragment makes through.

Fix it properly declaring the dependency on Kconfig and registering
netfilter hooks on ip_vs_add_service() and ip_vs_new_dest().

Reported-by: Li Shuang <shuali@redhat.com>
Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
Changes since v1:
- Move nf_defrag_ipv6_enable() call from __ip_vs_init() to
  ip_vs_new_dest() and ip_vs_add_service() for further optimization.
---
 net/netfilter/ipvs/Kconfig      |  1 +
 net/netfilter/ipvs/ip_vs_core.c | 10 ++++------
 net/netfilter/ipvs/ip_vs_ctl.c  | 10 ++++++++++
 3 files changed, 15 insertions(+), 6 deletions(-)

Comments

Julian Anastasov Feb. 11, 2019, 8:56 p.m. | #1
Hello,

On Mon, 11 Feb 2019, Andrea Claudi wrote:

> ipvs relies on nf_defrag_ipv6 module to manage IPv6 fragmentation,
> but lacks proper Kconfig dependencies and does not explicitly
> request defrag features.
> 
> As a result, if netfilter hooks are not loaded, when IPv6 fragmented
> packet are handled by ipvs only the first fragment makes through.
> 
> Fix it properly declaring the dependency on Kconfig and registering
> netfilter hooks on ip_vs_add_service() and ip_vs_new_dest().
> 
> Reported-by: Li Shuang <shuali@redhat.com>
> Signed-off-by: Andrea Claudi <aclaudi@redhat.com>

	Looks good to me, thanks!

Acked-by: Julian Anastasov <ja@ssi.bg>

> ---
> Changes since v1:
> - Move nf_defrag_ipv6_enable() call from __ip_vs_init() to
>   ip_vs_new_dest() and ip_vs_add_service() for further optimization.
> ---
>  net/netfilter/ipvs/Kconfig      |  1 +
>  net/netfilter/ipvs/ip_vs_core.c | 10 ++++------
>  net/netfilter/ipvs/ip_vs_ctl.c  | 10 ++++++++++
>  3 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/Kconfig b/net/netfilter/ipvs/Kconfig
> index cad48d07c818..8401cefd9f65 100644
> --- a/net/netfilter/ipvs/Kconfig
> +++ b/net/netfilter/ipvs/Kconfig
> @@ -29,6 +29,7 @@ config	IP_VS_IPV6
>  	bool "IPv6 support for IPVS"
>  	depends on IPV6 = y || IP_VS = IPV6
>  	select IP6_NF_IPTABLES
> +	select NF_DEFRAG_IPV6
>  	---help---
>  	  Add IPv6 support to IPVS.
>  
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index fe9abf3cc10a..235205c93e14 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -1536,14 +1536,12 @@ ip_vs_try_to_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
>  		/* sorry, all this trouble for a no-hit :) */
>  		IP_VS_DBG_PKT(12, af, pp, skb, iph->off,
>  			      "ip_vs_in: packet continues traversal as normal");
> -		if (iph->fragoffs) {
> -			/* Fragment that couldn't be mapped to a conn entry
> -			 * is missing module nf_defrag_ipv6
> -			 */
> -			IP_VS_DBG_RL("Unhandled frag, load nf_defrag_ipv6\n");
> +
> +		/* Fragment couldn't be mapped to a conn entry */
> +		if (iph->fragoffs)
>  			IP_VS_DBG_PKT(7, af, pp, skb, iph->off,
>  				      "unhandled fragment");
> -		}
> +
>  		*verdict = NF_ACCEPT;
>  		return 0;
>  	}
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 7d6318664eb2..86afacb07e5f 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -43,6 +43,7 @@
>  #ifdef CONFIG_IP_VS_IPV6
>  #include <net/ipv6.h>
>  #include <net/ip6_route.h>
> +#include <net/netfilter/ipv6/nf_defrag_ipv6.h>
>  #endif
>  #include <net/route.h>
>  #include <net/sock.h>
> @@ -895,6 +896,7 @@ ip_vs_new_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest,
>  {
>  	struct ip_vs_dest *dest;
>  	unsigned int atype, i;
> +	int ret = 0;
>  
>  	EnterFunction(2);
>  
> @@ -905,6 +907,10 @@ ip_vs_new_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest,
>  			atype & IPV6_ADDR_LINKLOCAL) &&
>  			!__ip_vs_addr_is_local_v6(svc->ipvs->net, &udest->addr.in6))
>  			return -EINVAL;
> +
> +		ret = nf_defrag_ipv6_enable(svc->ipvs->net);
> +		if (ret)
> +			return ret;
>  	} else
>  #endif
>  	{
> @@ -1228,6 +1234,10 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct ip_vs_service_user_kern *u,
>  			ret = -EINVAL;
>  			goto out_err;
>  		}
> +
> +		ret = nf_defrag_ipv6_enable(ipvs->net);
> +		if (ret)
> +			goto out_err;
>  	}
>  #endif
>  
> -- 
> 2.20.1

Regards

--
Julian Anastasov <ja@ssi.bg>
Simon Horman Feb. 12, 2019, 8:48 a.m. | #2
On Mon, Feb 11, 2019 at 04:14:39PM +0100, Andrea Claudi wrote:
> ipvs relies on nf_defrag_ipv6 module to manage IPv6 fragmentation,
> but lacks proper Kconfig dependencies and does not explicitly
> request defrag features.
> 
> As a result, if netfilter hooks are not loaded, when IPv6 fragmented
> packet are handled by ipvs only the first fragment makes through.
> 
> Fix it properly declaring the dependency on Kconfig and registering
> netfilter hooks on ip_vs_add_service() and ip_vs_new_dest().
> 
> Reported-by: Li Shuang <shuali@redhat.com>
> Signed-off-by: Andrea Claudi <aclaudi@redhat.com>

Thanks,

Pablo could you consider applying this to nf?

Acked-by: Simon Horman <horms@verge.net.au>

> ---
> Changes since v1:
> - Move nf_defrag_ipv6_enable() call from __ip_vs_init() to
>   ip_vs_new_dest() and ip_vs_add_service() for further optimization.
> ---
>  net/netfilter/ipvs/Kconfig      |  1 +
>  net/netfilter/ipvs/ip_vs_core.c | 10 ++++------
>  net/netfilter/ipvs/ip_vs_ctl.c  | 10 ++++++++++
>  3 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/net/netfilter/ipvs/Kconfig b/net/netfilter/ipvs/Kconfig
> index cad48d07c818..8401cefd9f65 100644
> --- a/net/netfilter/ipvs/Kconfig
> +++ b/net/netfilter/ipvs/Kconfig
> @@ -29,6 +29,7 @@ config	IP_VS_IPV6
>  	bool "IPv6 support for IPVS"
>  	depends on IPV6 = y || IP_VS = IPV6
>  	select IP6_NF_IPTABLES
> +	select NF_DEFRAG_IPV6
>  	---help---
>  	  Add IPv6 support to IPVS.
>  
> diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
> index fe9abf3cc10a..235205c93e14 100644
> --- a/net/netfilter/ipvs/ip_vs_core.c
> +++ b/net/netfilter/ipvs/ip_vs_core.c
> @@ -1536,14 +1536,12 @@ ip_vs_try_to_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
>  		/* sorry, all this trouble for a no-hit :) */
>  		IP_VS_DBG_PKT(12, af, pp, skb, iph->off,
>  			      "ip_vs_in: packet continues traversal as normal");
> -		if (iph->fragoffs) {
> -			/* Fragment that couldn't be mapped to a conn entry
> -			 * is missing module nf_defrag_ipv6
> -			 */
> -			IP_VS_DBG_RL("Unhandled frag, load nf_defrag_ipv6\n");
> +
> +		/* Fragment couldn't be mapped to a conn entry */
> +		if (iph->fragoffs)
>  			IP_VS_DBG_PKT(7, af, pp, skb, iph->off,
>  				      "unhandled fragment");
> -		}
> +
>  		*verdict = NF_ACCEPT;
>  		return 0;
>  	}
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 7d6318664eb2..86afacb07e5f 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -43,6 +43,7 @@
>  #ifdef CONFIG_IP_VS_IPV6
>  #include <net/ipv6.h>
>  #include <net/ip6_route.h>
> +#include <net/netfilter/ipv6/nf_defrag_ipv6.h>
>  #endif
>  #include <net/route.h>
>  #include <net/sock.h>
> @@ -895,6 +896,7 @@ ip_vs_new_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest,
>  {
>  	struct ip_vs_dest *dest;
>  	unsigned int atype, i;
> +	int ret = 0;
>  
>  	EnterFunction(2);
>  
> @@ -905,6 +907,10 @@ ip_vs_new_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest,
>  			atype & IPV6_ADDR_LINKLOCAL) &&
>  			!__ip_vs_addr_is_local_v6(svc->ipvs->net, &udest->addr.in6))
>  			return -EINVAL;
> +
> +		ret = nf_defrag_ipv6_enable(svc->ipvs->net);
> +		if (ret)
> +			return ret;
>  	} else
>  #endif
>  	{
> @@ -1228,6 +1234,10 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct ip_vs_service_user_kern *u,
>  			ret = -EINVAL;
>  			goto out_err;
>  		}
> +
> +		ret = nf_defrag_ipv6_enable(ipvs->net);
> +		if (ret)
> +			goto out_err;
>  	}
>  #endif
>  
> -- 
> 2.20.1
>
Pablo Neira Ayuso Feb. 12, 2019, 10:24 a.m. | #3
On Mon, Feb 11, 2019 at 04:14:39PM +0100, Andrea Claudi wrote:
> ipvs relies on nf_defrag_ipv6 module to manage IPv6 fragmentation,
> but lacks proper Kconfig dependencies and does not explicitly
> request defrag features.
> 
> As a result, if netfilter hooks are not loaded, when IPv6 fragmented
> packet are handled by ipvs only the first fragment makes through.
> 
> Fix it properly declaring the dependency on Kconfig and registering
> netfilter hooks on ip_vs_add_service() and ip_vs_new_dest().

Applied, thanks.

Patch

diff --git a/net/netfilter/ipvs/Kconfig b/net/netfilter/ipvs/Kconfig
index cad48d07c818..8401cefd9f65 100644
--- a/net/netfilter/ipvs/Kconfig
+++ b/net/netfilter/ipvs/Kconfig
@@ -29,6 +29,7 @@  config	IP_VS_IPV6
 	bool "IPv6 support for IPVS"
 	depends on IPV6 = y || IP_VS = IPV6
 	select IP6_NF_IPTABLES
+	select NF_DEFRAG_IPV6
 	---help---
 	  Add IPv6 support to IPVS.
 
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index fe9abf3cc10a..235205c93e14 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1536,14 +1536,12 @@  ip_vs_try_to_schedule(struct netns_ipvs *ipvs, int af, struct sk_buff *skb,
 		/* sorry, all this trouble for a no-hit :) */
 		IP_VS_DBG_PKT(12, af, pp, skb, iph->off,
 			      "ip_vs_in: packet continues traversal as normal");
-		if (iph->fragoffs) {
-			/* Fragment that couldn't be mapped to a conn entry
-			 * is missing module nf_defrag_ipv6
-			 */
-			IP_VS_DBG_RL("Unhandled frag, load nf_defrag_ipv6\n");
+
+		/* Fragment couldn't be mapped to a conn entry */
+		if (iph->fragoffs)
 			IP_VS_DBG_PKT(7, af, pp, skb, iph->off,
 				      "unhandled fragment");
-		}
+
 		*verdict = NF_ACCEPT;
 		return 0;
 	}
diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 7d6318664eb2..86afacb07e5f 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -43,6 +43,7 @@ 
 #ifdef CONFIG_IP_VS_IPV6
 #include <net/ipv6.h>
 #include <net/ip6_route.h>
+#include <net/netfilter/ipv6/nf_defrag_ipv6.h>
 #endif
 #include <net/route.h>
 #include <net/sock.h>
@@ -895,6 +896,7 @@  ip_vs_new_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest,
 {
 	struct ip_vs_dest *dest;
 	unsigned int atype, i;
+	int ret = 0;
 
 	EnterFunction(2);
 
@@ -905,6 +907,10 @@  ip_vs_new_dest(struct ip_vs_service *svc, struct ip_vs_dest_user_kern *udest,
 			atype & IPV6_ADDR_LINKLOCAL) &&
 			!__ip_vs_addr_is_local_v6(svc->ipvs->net, &udest->addr.in6))
 			return -EINVAL;
+
+		ret = nf_defrag_ipv6_enable(svc->ipvs->net);
+		if (ret)
+			return ret;
 	} else
 #endif
 	{
@@ -1228,6 +1234,10 @@  ip_vs_add_service(struct netns_ipvs *ipvs, struct ip_vs_service_user_kern *u,
 			ret = -EINVAL;
 			goto out_err;
 		}
+
+		ret = nf_defrag_ipv6_enable(ipvs->net);
+		if (ret)
+			goto out_err;
 	}
 #endif