Skip to contentSkip to conversation

netfilter: ipset: fix linking with CONFIG_IPV6=n

PATCH Patrick McHardy 2 participants 5 messages Feb 2, 2011 – Feb 3, 2011

Subsystems (2)
Networking netfilter
lore
Share & tools
lore

Patrick McHardy

author
#0 lore raw

Add some #ifdefs to unconditionally return false in ip_set_get_ip6_port() when CONFIG_IPV6=n and convert to ipv6_skip_exthdr() to avoid pulling in the ip6_tables module when loading ipset.

A slightly more code-saving alternative would be to add a dummy inline function or ifdef the callsites. Please let me know which way you prefer.

net/netfilter/ipset/ip_set_getport.c +11 -6
index 4dd2785..5ea0736 100644
@@ -13,6 +13,7 @@
 #include <linux/icmpv6.h>
 #include <linux/netfilter_ipv6/ip6_tables.h>
 #include <net/ip.h>
+#include <net/ipv6.h>
 
 #include <linux/netfilter/ipset/ip_set_getport.h>
 

@@ -97,15 +98,19 @@ bool
 ip_set_get_ip6_port(const struct sk_buff *skb, bool src,
 		    __be16 *port, u8 *proto)
 {
-	unsigned int protooff = 0;
-	int protocol;
-	unsigned short fragoff;
+#if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
+	int protoff;
+	u8 nexthdr;
 
-	protocol = ipv6_find_hdr(skb, &protooff, -1, &fragoff);
-	if (protocol <= 0 || fragoff)
+	nexthdr = ipv6_hdr(skb)->nexthdr;
+	protoff = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &nexthdr);
+	if (protoff < 0)
 		return false;
 
-	return get_port(skb, protocol, protooff, src, port, proto);
+	return get_port(skb, nexthdr, protoff, src, port, proto);
+#else
+	return false;
+#endif
 }
 EXPORT_SYMBOL_GPL(ip_set_get_ip6_port);
author
#1 lore raw

[resend with netfilter-devel CCed instead of netfilter]

Quoted 9 lines · Patrick McHardy

Add some #ifdefs to unconditionally return false in ip_set_get_ip6_port() when CONFIG_IPV6=n and convert to ipv6_skip_exthdr() to avoid pulling in the ip6_tables module when loading ipset.

A slightly more code-saving alternative would be to add a dummy inline function or ifdef the callsites. Please let me know which way you prefer.

net/netfilter/ipset/ip_set_getport.c +11 -6
index 4dd2785..5ea0736 100644
@@ -13,6 +13,7 @@
 #include <linux/icmpv6.h>
 #include <linux/netfilter_ipv6/ip6_tables.h>
 #include <net/ip.h>
+#include <net/ipv6.h>
 
 #include <linux/netfilter/ipset/ip_set_getport.h>
 

@@ -97,15 +98,19 @@ bool
 ip_set_get_ip6_port(const struct sk_buff *skb, bool src,
 		    __be16 *port, u8 *proto)
 {
-	unsigned int protooff = 0;
-	int protocol;
-	unsigned short fragoff;
+#if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
+	int protoff;
+	u8 nexthdr;
 
-	protocol = ipv6_find_hdr(skb, &protooff, -1, &fragoff);
-	if (protocol <= 0 || fragoff)
+	nexthdr = ipv6_hdr(skb)->nexthdr;
+	protoff = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &nexthdr);
+	if (protoff < 0)
 		return false;
 
-	return get_port(skb, protocol, protooff, src, port, proto);
+	return get_port(skb, nexthdr, protoff, src, port, proto);
+#else
+	return false;
+#endif
 }
 EXPORT_SYMBOL_GPL(ip_set_get_ip6_port);
#2 lore raw
In reply to Patrick McHardy

Hi Patrick,

Quoted 4 lines · Patrick McHardy

Add some #ifdefs to unconditionally return false in ip_set_get_ip6_port() when CONFIG_IPV6=n and convert to ipv6_skip_exthdr() to avoid pulling in the ip6_tables module when loading ipset.

You left out checking fragmented packets. Is it intentional?

Quoted 3 lines

A slightly more code-saving alternative would be to add a dummy inline function or ifdef the callsites. Please let me know which way you prefer.

A dummy inline looks nicer to me, because that way the IPv6 and non-IPv6 cases were clearly separated.

Best regards,
Jozsef
-
E-mail : [email protected], [email protected] PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics

          H-1525 Budapest 114, POB. 49, Hungary
author
#3 lore raw

Am 02.02.2011 20:52, schrieb Jozsef Kadlecsik:

Quoted 10 lines

Hi Patrick,

Quoted 4 lines · Patrick McHardy

Add some #ifdefs to unconditionally return false in ip_set_get_ip6_port() when CONFIG_IPV6=n and convert to ipv6_skip_exthdr() to avoid pulling in the ip6_tables module when loading ipset.

You left out checking fragmented packets. Is it intentional?

Yes, ipv6_skip_exthdr() returns an error for non-first fragments. For the first fragment skb_header_pointer() checks that enough data for the port numbers is present.

Quoted 6 lines
Quoted 3 lines

A slightly more code-saving alternative would be to add a dummy inline function or ifdef the callsites. Please let me know which way you prefer.

A dummy inline looks nicer to me, because that way the IPv6 and non-IPv6 cases were clearly separated.

OK, I'm going to commit this patch.

include/linux/netfilter/ipset/ip_set_getport.h +10 -0
index 694c433..3882a81 100644
@@ -3,8 +3,18 @@
 
 extern bool ip_set_get_ip4_port(const struct sk_buff *skb, bool src,
 				__be16 *port, u8 *proto);
+
+#if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
 extern bool ip_set_get_ip6_port(const struct sk_buff *skb, bool src,
 				__be16 *port, u8 *proto);
+#else
+static inline bool ip_set_get_ip6_port(const struct sk_buff *skb, bool src,
+				       __be16 *port, u8 *proto)
+{
+	return false;
+}
+#endif
+
 extern bool ip_set_get_ip_port(const struct sk_buff *skb, u8 pf, bool src,
 				__be16 *port);
net/netfilter/ipset/ip_set_getport.c +9 -6
index 4dd2785..8d52272 100644
@@ -13,6 +13,7 @@
 #include <linux/icmpv6.h>
 #include <linux/netfilter_ipv6/ip6_tables.h>
 #include <net/ip.h>
+#include <net/ipv6.h>
 
 #include <linux/netfilter/ipset/ip_set_getport.h>
 

@@ -93,21 +94,23 @@ ip_set_get_ip4_port(const struct sk_buff *skb, bool src,
 }
 EXPORT_SYMBOL_GPL(ip_set_get_ip4_port);
 
+#if defined(CONFIG_IP6_NF_IPTABLES) || defined(CONFIG_IP6_NF_IPTABLES_MODULE)
 bool
 ip_set_get_ip6_port(const struct sk_buff *skb, bool src,
 		    __be16 *port, u8 *proto)
 {
-	unsigned int protooff = 0;
-	int protocol;
-	unsigned short fragoff;
+	int protoff;
+	u8 nexthdr;
 
-	protocol = ipv6_find_hdr(skb, &protooff, -1, &fragoff);
-	if (protocol <= 0 || fragoff)
+	nexthdr = ipv6_hdr(skb)->nexthdr;
+	protoff = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &nexthdr);
+	if (protoff < 0)
 		return false;
 
-	return get_port(skb, protocol, protooff, src, port, proto);
+	return get_port(skb, nexthdr, protoff, src, port, proto);
 }
 EXPORT_SYMBOL_GPL(ip_set_get_ip6_port);
+#endif
 
 bool
 ip_set_get_ip_port(const struct sk_buff *skb, u8 pf, bool src, __be16 *port)
#4 lore raw
Quoted 16 lines · Patrick McHardy

Am 02.02.2011 20:52, schrieb Jozsef Kadlecsik:

Quoted 10 lines

Hi Patrick,

Quoted 4 lines · Patrick McHardy

Add some #ifdefs to unconditionally return false in ip_set_get_ip6_port() when CONFIG_IPV6=n and convert to ipv6_skip_exthdr() to avoid pulling in the ip6_tables module when loading ipset.

You left out checking fragmented packets. Is it intentional?

Yes, ipv6_skip_exthdr() returns an error for non-first fragments. For the first fragment skb_header_pointer() checks that enough data for the port numbers is present.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

A slightly more code-saving alternative would be to add a dummy inline function or ifdef the callsites. Please let me know which way you prefer.

A dummy inline looks nicer to me, because that way the IPv6 and non-IPv6 cases were clearly separated.

OK, I'm going to commit this patch.

I committed it in my tree too. Thanks!

Best regards,
Jozsef
-
E-mail : [email protected], [email protected] PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : KFKI Research Institute for Particle and Nuclear Physics

          H-1525 Budapest 114, POB. 49, Hungary