[NETFILTER 00/07]: Netfilter fixes

[NETFILTER 00/07]: Netfilter fixes

[Patrick McHardy]

Hi Dave,

following are a few netfilter fixes for 2.6.25, fixing a BUG() when
mangling non-linear skbs in {ip,ip6,nfnetlink}_queue, a compilation
error in busybox, IPv6 address range matching in xt_iprange, missing
unidef'ing of xt_policy.h. Additionally there is a patch to remove
an unnecessary member from the xt_hashlimit matchinfo, which we need
to do know before it hits a release.

Please apply, thanks.


 include/linux/netfilter.h              |    2 +-
 include/linux/netfilter/Kbuild         |    2 +-
 include/linux/netfilter/xt_hashlimit.h |    1 -
 net/bridge/netfilter/ebt_dnat.c        |    2 +-
 net/bridge/netfilter/ebt_redirect.c    |    2 +-
 net/bridge/netfilter/ebt_snat.c        |    2 +-
 net/ipv4/netfilter/arpt_mangle.c       |    2 +-
 net/ipv4/netfilter/ip_queue.c          |   12 +++++++-----
 net/ipv6/netfilter/ip6_queue.c         |   10 ++++++----
 net/netfilter/nfnetlink_queue.c        |   10 ++++++----
 net/netfilter/xt_hashlimit.c           |    3 ---
 net/netfilter/xt_iprange.c             |    2 +-
 net/netfilter/xt_u32.c                 |   11 +++++------
 13 files changed, 31 insertions(+), 30 deletions(-)

Jan Engelhardt (2):
      [NETFILTER]: xt_hashlimit: remove unneeded struct member
      [NETFILTER]: xt_iprange: fix subtraction-based comparison

Joonwoo Park (1):
      [NETFILTER]: Fix incorrect use of skb_make_writable

Patrick McHardy (2):
      [NETFILTER]: {ip,ip6,nfnetlink}_queue: fix SKB_LINEAR_ASSERT when mangling packet data
      [NETFILTER]: Use __u32 in struct nf_inet_addr

Pavel Emelyanov (1):
      [NETFILTER]: xt_u32: drop the actually unused variable from u32_match_it

Robert P. J. Day (1):
      [NETFILTER]: Make sure xt_policy.h is unifdef'ed.

[NETFILTER 01/07]: nfnetlink_queue: fix SKB_LINEAR_ASSERT when mangling packet data

[Patrick McHardy]

[NETFILTER]: {ip,ip6,nfnetlink}_queue: fix SKB_LINEAR_ASSERT when mangling packet data

As reported by Tomas Simonaitis <tomas.simonaitis@gmail.com>, inserting new
data in skbs queued over {ip,ip6,nfnetlink}_queue triggers a SKB_LINEAR_ASSERT
in skb_put().

Going back through the git history, it seems this bug is present since at
least 2.6.12-rc2, probably even since the removal of skb_linearize() for
netfilter.

Linearize non-linear skbs through skb_copy_expand() when enlarging them.
Tested by Thomas, fixes bugzilla #9933.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 2292fd9623ce0391217dfe8cec57f70fd90da538
tree ba852b5da64fac4aaec85ecf9622cca31e3ffb0d
parent 73b9987a76c8e0b5326afcc3632f79be425676bc
author Patrick McHardy <kaber@trash.net> Tue, 19 Feb 2008 17:15:26 +0100
committer Patrick McHardy <kaber@trash.net> Tue, 19 Feb 2008 17:15:26 +0100

 net/ipv4/netfilter/ip_queue.c   |   12 +++++++-----
 net/ipv6/netfilter/ip6_queue.c  |   10 ++++++----
 net/netfilter/nfnetlink_queue.c |   10 ++++++----
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/net/ipv4/netfilter/ip_queue.c b/net/ipv4/netfilter/ip_queue.c
index 6bda110..fe05da4 100644
--- a/net/ipv4/netfilter/ip_queue.c
+++ b/net/ipv4/netfilter/ip_queue.c
@@ -283,8 +283,8 @@ static int
 ipq_mangle_ipv4(ipq_verdict_msg_t *v, struct nf_queue_entry *e)
 {
 	int diff;
-	int err;
 	struct iphdr *user_iph = (struct iphdr *)v->payload;
+	struct sk_buff *nskb;
 
 	if (v->data_len < sizeof(*user_iph))
 		return 0;
@@ -296,14 +296,16 @@ ipq_mangle_ipv4(ipq_verdict_msg_t *v, struct nf_queue_entry *e)
 		if (v->data_len > 0xFFFF)
 			return -EINVAL;
 		if (diff > skb_tailroom(e->skb)) {
-			err = pskb_expand_head(e->skb, 0,
+			nskb = skb_copy_expand(e->skb, 0,
 					       diff - skb_tailroom(e->skb),
 					       GFP_ATOMIC);
-			if (err) {
+			if (!nskb) {
 				printk(KERN_WARNING "ip_queue: error "
-				      "in mangle, dropping packet: %d\n", -err);
-				return err;
+				      "in mangle, dropping packet\n");
+				return -ENOMEM;
 			}
+			kfree_skb(e->skb);
+			e->skb = nskb;
 		}
 		skb_put(e->skb, diff);
 	}
diff --git a/net/ipv6/netfilter/ip6_queue.c b/net/ipv6/netfilter/ip6_queue.c
index e869916..cc2f9af 100644
--- a/net/ipv6/netfilter/ip6_queue.c
+++ b/net/ipv6/netfilter/ip6_queue.c
@@ -285,8 +285,8 @@ static int
 ipq_mangle_ipv6(ipq_verdict_msg_t *v, struct nf_queue_entry *e)
 {
 	int diff;
-	int err;
 	struct ipv6hdr *user_iph = (struct ipv6hdr *)v->payload;
+	struct sk_buff *nskb;
 
 	if (v->data_len < sizeof(*user_iph))
 		return 0;
@@ -298,14 +298,16 @@ ipq_mangle_ipv6(ipq_verdict_msg_t *v, struct nf_queue_entry *e)
 		if (v->data_len > 0xFFFF)
 			return -EINVAL;
 		if (diff > skb_tailroom(e->skb)) {
-			err = pskb_expand_head(e->skb, 0,
+			nskb = skb_copy_expand(e->skb, 0,
 					       diff - skb_tailroom(e->skb),
 					       GFP_ATOMIC);
-			if (err) {
+			if (!nskb) {
 				printk(KERN_WARNING "ip6_queue: OOM "
 				      "in mangle, dropping packet\n");
-				return err;
+				return -ENOMEM;
 			}
+			kfree_skb(e->skb);
+			e->skb = nskb;
 		}
 		skb_put(e->skb, diff);
 	}
diff --git a/net/netfilter/nfnetlink_queue.c b/net/netfilter/nfnetlink_queue.c
index a48b20f..0043d3a 100644
--- a/net/netfilter/nfnetlink_queue.c
+++ b/net/netfilter/nfnetlink_queue.c
@@ -443,8 +443,8 @@ err_out:
 static int
 nfqnl_mangle(void *data, int data_len, struct nf_queue_entry *e)
 {
+	struct sk_buff *nskb;
 	int diff;
-	int err;
 
 	diff = data_len - e->skb->len;
 	if (diff < 0) {
@@ -454,14 +454,16 @@ nfqnl_mangle(void *data, int data_len, struct nf_queue_entry *e)
 		if (data_len > 0xFFFF)
 			return -EINVAL;
 		if (diff > skb_tailroom(e->skb)) {
-			err = pskb_expand_head(e->skb, 0,
+			nskb = skb_copy_expand(e->skb, 0,
 					       diff - skb_tailroom(e->skb),
 					       GFP_ATOMIC);
-			if (err) {
+			if (!nskb) {
 				printk(KERN_WARNING "nf_queue: OOM "
 				      "in mangle, dropping packet\n");
-				return err;
+				return -ENOMEM;
 			}
+			kfree_skb(e->skb);
+			e->skb = nskb;
 		}
 		skb_put(e->skb, diff);
 	}

[NETFILTER 02/07]: xt_u32: drop the actually unused variable from u32_match_it

[Patrick McHardy]

[NETFILTER]: xt_u32: drop the actually unused variable from u32_match_it

The int ret variable is used only to trigger the BUG_ON() after
the skb_copy_bits() call, so check the call failure directly
and drop the variable.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 30c709c51fa0f400859890d534cfa02eb8d06cc7
tree 6df1f3fd873cae02ae066e2a76bafd1ef32e055a
parent 2292fd9623ce0391217dfe8cec57f70fd90da538
author Pavel Emelyanov <xemul@openvz.org> Tue, 19 Feb 2008 17:15:26 +0100
committer Patrick McHardy <kaber@trash.net> Tue, 19 Feb 2008 17:15:26 +0100

 net/netfilter/xt_u32.c |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/xt_u32.c b/net/netfilter/xt_u32.c
index 9b8ed39..627e0f3 100644
--- a/net/netfilter/xt_u32.c
+++ b/net/netfilter/xt_u32.c
@@ -26,7 +26,6 @@ static bool u32_match_it(const struct xt_u32 *data,
 	u_int32_t pos;
 	u_int32_t val;
 	u_int32_t at;
-	int ret;
 
 	/*
 	 * Small example: "0 >> 28 == 4 && 8 & 0xFF0000 >> 16 = 6, 17"
@@ -40,8 +39,8 @@ static bool u32_match_it(const struct xt_u32 *data,
 		if (skb->len < 4 || pos > skb->len - 4)
 			return false;
 
-		ret   = skb_copy_bits(skb, pos, &n, sizeof(n));
-		BUG_ON(ret < 0);
+		if (skb_copy_bits(skb, pos, &n, sizeof(n)) < 0)
+			BUG();
 		val   = ntohl(n);
 		nnums = ct->nnums;
 
@@ -67,9 +66,9 @@ static bool u32_match_it(const struct xt_u32 *data,
 				    pos > skb->len - at - 4)
 					return false;
 
-				ret = skb_copy_bits(skb, at + pos, &n,
-						    sizeof(n));
-				BUG_ON(ret < 0);
+				if (skb_copy_bits(skb, at + pos, &n,
+						    sizeof(n)) < 0)
+					BUG();
 				val = ntohl(n);
 				break;
 			}

[NETFILTER 03/07]: Fix incorrect use of skb_make_writable

[Patrick McHardy]

[NETFILTER]: Fix incorrect use of skb_make_writable

http://bugzilla.kernel.org/show_bug.cgi?id=9920
The function skb_make_writable returns true or false.

Signed-off-by: Joonwoo Park <joonwpark81@gmail.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 02413e566cd9a49bcf1f15f69cdc749780181412
tree 9ec6b55a428fbfdd24da2bc619ebf4c5dabb0354
parent 30c709c51fa0f400859890d534cfa02eb8d06cc7
author Joonwoo Park <joonwpark81@gmail.com> Tue, 19 Feb 2008 17:15:27 +0100
committer Patrick McHardy <kaber@trash.net> Tue, 19 Feb 2008 17:15:27 +0100

 net/bridge/netfilter/ebt_dnat.c     |    2 +-
 net/bridge/netfilter/ebt_redirect.c |    2 +-
 net/bridge/netfilter/ebt_snat.c     |    2 +-
 net/ipv4/netfilter/arpt_mangle.c    |    2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/bridge/netfilter/ebt_dnat.c b/net/bridge/netfilter/ebt_dnat.c
index e700cbf..1ec671d 100644
--- a/net/bridge/netfilter/ebt_dnat.c
+++ b/net/bridge/netfilter/ebt_dnat.c
@@ -20,7 +20,7 @@ static int ebt_target_dnat(struct sk_buff *skb, unsigned int hooknr,
 {
 	const struct ebt_nat_info *info = data;
 
-	if (skb_make_writable(skb, 0))
+	if (!skb_make_writable(skb, 0))
 		return NF_DROP;
 
 	memcpy(eth_hdr(skb)->h_dest, info->mac, ETH_ALEN);
diff --git a/net/bridge/netfilter/ebt_redirect.c b/net/bridge/netfilter/ebt_redirect.c
index bfdf2fb..bfb9f74 100644
--- a/net/bridge/netfilter/ebt_redirect.c
+++ b/net/bridge/netfilter/ebt_redirect.c
@@ -21,7 +21,7 @@ static int ebt_target_redirect(struct sk_buff *skb, unsigned int hooknr,
 {
 	const struct ebt_redirect_info *info = data;
 
-	if (skb_make_writable(skb, 0))
+	if (!skb_make_writable(skb, 0))
 		return NF_DROP;
 
 	if (hooknr != NF_BR_BROUTING)
diff --git a/net/bridge/netfilter/ebt_snat.c b/net/bridge/netfilter/ebt_snat.c
index e252dab..204f996 100644
--- a/net/bridge/netfilter/ebt_snat.c
+++ b/net/bridge/netfilter/ebt_snat.c
@@ -22,7 +22,7 @@ static int ebt_target_snat(struct sk_buff *skb, unsigned int hooknr,
 {
 	const struct ebt_nat_info *info = data;
 
-	if (skb_make_writable(skb, 0))
+	if (!skb_make_writable(skb, 0))
 		return NF_DROP;
 
 	memcpy(eth_hdr(skb)->h_source, info->mac, ETH_ALEN);
diff --git a/net/ipv4/netfilter/arpt_mangle.c b/net/ipv4/netfilter/arpt_mangle.c
index 45fa4e2..3f4222b 100644
--- a/net/ipv4/netfilter/arpt_mangle.c
+++ b/net/ipv4/netfilter/arpt_mangle.c
@@ -19,7 +19,7 @@ target(struct sk_buff *skb,
 	unsigned char *arpptr;
 	int pln, hln;
 
-	if (skb_make_writable(skb, skb->len))
+	if (!skb_make_writable(skb, skb->len))
 		return NF_DROP;
 
 	arp = arp_hdr(skb);

[NETFILTER 04/07]: Make sure xt_policy.h is unifdef'ed.

[Patrick McHardy]

[NETFILTER]: Make sure xt_policy.h is unifdef'ed.

Since the header file xt_policy.h tests __KERNEL__, it should be
unifdef'ed before exporting to userspace.

Signed-off-by: Robert P. J. Day <rpjday@crashcourse.ca>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 0776e52cdb04aad5b59c4e12dc16e7b58737c2d2
tree d8f3cb734bad2c4df073262ab9ac14989f28eb28
parent 02413e566cd9a49bcf1f15f69cdc749780181412
author Robert P. J. Day <rpjday@crashcourse.ca> Tue, 19 Feb 2008 17:15:28 +0100
committer Patrick McHardy <kaber@trash.net> Tue, 19 Feb 2008 17:15:28 +0100

 include/linux/netfilter/Kbuild |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/netfilter/Kbuild b/include/linux/netfilter/Kbuild
index 91fef0c..3aff513 100644
--- a/include/linux/netfilter/Kbuild
+++ b/include/linux/netfilter/Kbuild
@@ -30,7 +30,6 @@ header-y += xt_mark.h
 header-y += xt_multiport.h
 header-y += xt_owner.h
 header-y += xt_pkttype.h
-header-y += xt_policy.h
 header-y += xt_rateest.h
 header-y += xt_realm.h
 header-y += xt_sctp.h
@@ -47,3 +46,4 @@ unifdef-y += nfnetlink.h
 unifdef-y += nfnetlink_compat.h
 unifdef-y += x_tables.h
 unifdef-y += xt_physdev.h
+unifdef-y += xt_policy.h

[NETFILTER 05/07]: xt_hashlimit: remove unneeded struct member

[Patrick McHardy]

[NETFILTER]: xt_hashlimit: remove unneeded struct member

By allocating ->hinfo, we already have the needed indirection to cope
with the per-cpu xtables struct match_entry.

[Patrick: do this now before the revision 1 struct is used by userspace]

Signed-off-by: Jan Engelhardt <jengelh@computergmbh.de>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 4e2e7aba6471fcd945a3f8fdc40d5cf11e7039b3
tree fee4fdd8dfbf77ac747e89a316b591fab09bf884
parent 0776e52cdb04aad5b59c4e12dc16e7b58737c2d2
author Jan Engelhardt <jengelh@computergmbh.de> Tue, 19 Feb 2008 17:15:28 +0100
committer Patrick McHardy <kaber@trash.net> Tue, 19 Feb 2008 17:15:28 +0100

 include/linux/netfilter/xt_hashlimit.h |    1 -
 net/netfilter/xt_hashlimit.c           |    3 ---
 2 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/include/linux/netfilter/xt_hashlimit.h b/include/linux/netfilter/xt_hashlimit.h
index 58b818e..51b18d8 100644
--- a/include/linux/netfilter/xt_hashlimit.h
+++ b/include/linux/netfilter/xt_hashlimit.h
@@ -61,7 +61,6 @@ struct xt_hashlimit_mtinfo1 {
 
 	/* Used internally by the kernel */
 	struct xt_hashlimit_htable *hinfo __attribute__((aligned(8)));
-	struct xt_hashlimit_mtinfo1 *master __attribute__((aligned(8)));
 };
 
 #endif /*_XT_HASHLIMIT_H*/
diff --git a/net/netfilter/xt_hashlimit.c b/net/netfilter/xt_hashlimit.c
index 744c7f2..5418ce5 100644
--- a/net/netfilter/xt_hashlimit.c
+++ b/net/netfilter/xt_hashlimit.c
@@ -774,9 +774,6 @@ hashlimit_mt_check(const char *tablename, const void *inf,
 		return false;
 	}
 	mutex_unlock(&hlimit_mutex);
-
-	/* Ugly hack: For SMP, we only want to use one set */
-	info->master = info;
 	return true;
 }
 

[NETFILTER 06/07]: xt_iprange: fix subtraction-based comparison

[Patrick McHardy]

[NETFILTER]: xt_iprange: fix subtraction-based comparison

The host address parts need to be converted to host-endian first
before arithmetic makes any sense on them.

Signed-off-by: Jan Engelhardt <jengelh@computergmbh.de>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 5a090319a35aac1052102d97c431230d3140daf1
tree 72fb9bcb9ebf94fad277a3461288d23841f50662
parent 4e2e7aba6471fcd945a3f8fdc40d5cf11e7039b3
author Jan Engelhardt <jengelh@computergmbh.de> Tue, 19 Feb 2008 17:15:29 +0100
committer Patrick McHardy <kaber@trash.net> Tue, 19 Feb 2008 17:15:29 +0100

 net/netfilter/xt_iprange.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/netfilter/xt_iprange.c b/net/netfilter/xt_iprange.c
index 4f984dc..500528d 100644
--- a/net/netfilter/xt_iprange.c
+++ b/net/netfilter/xt_iprange.c
@@ -102,7 +102,7 @@ iprange_ipv6_sub(const struct in6_addr *a, const struct in6_addr *b)
 	int r;
 
 	for (i = 0; i < 4; ++i) {
-		r = (__force u32)a->s6_addr32[i] - (__force u32)b->s6_addr32[i];
+		r = ntohl(a->s6_addr32[i]) - ntohl(b->s6_addr32[i]);
 		if (r != 0)
 			return r;
 	}

[NETFILTER 07/07]: Use __u32 in struct nf_inet_addr

[Patrick McHardy]

[NETFILTER]: Use __u32 in struct nf_inet_addr

As reported by David Woodhouse <dwmw2@infradead.org>, using u_int32_t in
struct nf_inet_addr breaks the busybox build. Fix by using __u32.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 4f4e9be33da8cc1a297c4d7ef51c1fa1effcbace
tree a856969ae8edeebca7c8018e18e4304cd198e529
parent 5a090319a35aac1052102d97c431230d3140daf1
author Patrick McHardy <kaber@trash.net> Tue, 19 Feb 2008 17:15:29 +0100
committer Patrick McHardy <kaber@trash.net> Tue, 19 Feb 2008 17:15:29 +0100

 include/linux/netfilter.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index d74e79b..b74b615 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -51,7 +51,7 @@ enum nf_inet_hooks {
 };
 
 union nf_inet_addr {
-	u_int32_t	all[4];
+	__u32		all[4];
 	__be32		ip;
 	__be32		ip6[4];
 	struct in_addr	in;

Re: [NETFILTER 07/07]: Use __u32 in struct nf_inet_addr

[Jan Engelhardt]

On Feb 19 2008 17:25, Patrick McHardy wrote:
>[NETFILTER]: Use __u32 in struct nf_inet_addr
>
>As reported by David Woodhouse <dwmw2@infradead.org>, using u_int32_t in
>struct nf_inet_addr breaks the busybox build. Fix by using __u32.

... that's why I say we should use uint32_t (with just one underscore),
because that is available in userspace thorugh <stdint.h>,
while u_int32_t is not (at first glance²).


² db1/db.h and ALSA do seem to use it, though.
-
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [NETFILTER 07/07]: Use __u32 in struct nf_inet_addr

[Patrick McHardy]

Jan Engelhardt wrote:
> On Feb 19 2008 17:25, Patrick McHardy wrote:
>> [NETFILTER]: Use __u32 in struct nf_inet_addr
>>
>> As reported by David Woodhouse <dwmw2@infradead.org>, using u_int32_t in
>> struct nf_inet_addr breaks the busybox build. Fix by using __u32.
> 
> ... that's why I say we should use uint32_t (with just one underscore),
> because that is available in userspace thorugh <stdint.h>,
> while u_int32_t is not (at first glance²).


As David explained, kernel headers shouldn't assume that userspace
has pulled in any of these typedefs and therefore should use
the kernel-internally defined types.

-
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [NETFILTER 00/07]: Netfilter fixes

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Tue, 19 Feb 2008 17:25:44 +0100 (MET)

> following are a few netfilter fixes for 2.6.25, fixing a BUG() when
> mangling non-linear skbs in {ip,ip6,nfnetlink}_queue, a compilation
> error in busybox, IPv6 address range matching in xt_iprange, missing
> unidef'ing of xt_policy.h. Additionally there is a patch to remove
> an unnecessary member from the xt_hashlimit matchinfo, which we need
> to do know before it hits a release.
> 
> Please apply, thanks.

All applied, thanks Patrick.