[NETFILTER 00/05]: Netfilter fixes

[NETFILTER 00/05]: Netfilter fixes

[Patrick McHardy]

Hi Dave,

these patches for 2.6.25 fix a couple of netfilter bugs: the
smp_processor_id() warning when using preemptible RCU reported
by multiple people, address and state matching in the new
xt_conntrack revision, and improper use of parenthesis in
the NF_QUEUE_NR macro.

Additionally there is a patch to make the NAT core behave similar
to the recently removed SAME target for SNAT, which fixes problems
when accesing certain multihomed sites.

Please apply, thanks.


 include/linux/netfilter.h         |    2 +-
 net/ipv4/netfilter/nf_nat_core.c  |   11 +++++++----
 net/netfilter/nf_conntrack_core.c |   15 ++++++++++++---
 net/netfilter/xt_conntrack.c      |    4 ++--
 4 files changed, 22 insertions(+), 10 deletions(-)

Jan Engelhardt (2):
      [NETFILTER]: xt_conntrack: fix missing boolean clamping
      [NETFILTER]: xt_conntrack: fix IPv4 address comparison

Patrick McHardy (3):
      [NETFILTER]: nf_conntrack: fix smp_processor_id() in preemptible code warning
      [NETFILTER]: nf_nat: always select same SNAT source for same host
      [NETFILTER]: Fix NF_QUEUE_NR() parenthesis

[NETFILTER 01/05]: nf_conntrack: fix smp_processor_id() in preemptible code warning

[Patrick McHardy]

[NETFILTER]: nf_conntrack: fix smp_processor_id() in preemptible code warning

Since we're using RCU for the conntrack hash now, we need to avoid
getting preempted or interrupted by BHs while changing the stats.

Fixes warning reported by Tilman Schmidt <tilman@imap.cc> when using
preemptible RCU:

[   48.180297] BUG: using smp_processor_id() in preemptible [00000000] code: ntpdate/3562
[   48.180297] caller is __nf_conntrack_find+0x9b/0xeb [nf_conntrack]
[   48.180297] Pid: 3562, comm: ntpdate Not tainted 2.6.25-rc2-mm1-testing #1
[   48.180297]  [<c02015b9>] debug_smp_processor_id+0x99/0xb0
[   48.180297]  [<fac643a7>] __nf_conntrack_find+0x9b/0xeb [nf_conntrack]

Tested-by: Tilman Schmidt <tilman@imap.cc>
Tested-by: Christian Casteyde <casteyde.christian@free.fr> [Bugzilla #10097]

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

---
commit 2ffa8857857148a5b0c0823852d6cb2551a3ae5a
tree 62ff238b8f59b581332c36f703eb4a8878377fae
parent fccf186fa8fba308f4a478691e86399336488dd1
author Patrick McHardy <kaber@trash.net> Wed, 27 Feb 2008 13:23:55 +0100
committer Patrick McHardy <kaber@trash.net> Wed, 27 Feb 2008 13:23:55 +0100

 net/netfilter/nf_conntrack_core.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 327e847..b77eb56 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -256,13 +256,19 @@ __nf_conntrack_find(const struct nf_conntrack_tuple *tuple)
 	struct hlist_node *n;
 	unsigned int hash = hash_conntrack(tuple);
 
+	/* Disable BHs the entire time since we normally need to disable them
+	 * at least once for the stats anyway.
+	 */
+	local_bh_disable();
 	hlist_for_each_entry_rcu(h, n, &nf_conntrack_hash[hash], hnode) {
 		if (nf_ct_tuple_equal(tuple, &h->tuple)) {
 			NF_CT_STAT_INC(found);
+			local_bh_enable();
 			return h;
 		}
 		NF_CT_STAT_INC(searched);
 	}
+	local_bh_enable();
 
 	return NULL;
 }
@@ -400,17 +406,20 @@ nf_conntrack_tuple_taken(const struct nf_conntrack_tuple *tuple,
 	struct hlist_node *n;
 	unsigned int hash = hash_conntrack(tuple);
 
-	rcu_read_lock();
+	/* Disable BHs the entire time since we need to disable them at
+	 * least once for the stats anyway.
+	 */
+	rcu_read_lock_bh();
 	hlist_for_each_entry_rcu(h, n, &nf_conntrack_hash[hash], hnode) {
 		if (nf_ct_tuplehash_to_ctrack(h) != ignored_conntrack &&
 		    nf_ct_tuple_equal(tuple, &h->tuple)) {
 			NF_CT_STAT_INC(found);
-			rcu_read_unlock();
+			rcu_read_unlock_bh();
 			return 1;
 		}
 		NF_CT_STAT_INC(searched);
 	}
-	rcu_read_unlock();
+	rcu_read_unlock_bh();
 
 	return 0;
 }

Re: [NETFILTER 01/05]: nf_conntrack: fix smp_processor_id() in preemptible code warning

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Wed, 27 Feb 2008 14:14:20 +0100 (MET)

> [NETFILTER]: nf_conntrack: fix smp_processor_id() in preemptible code warning
> 
> Since we're using RCU for the conntrack hash now, we need to avoid
> getting preempted or interrupted by BHs while changing the stats.
> 
> Fixes warning reported by Tilman Schmidt <tilman@imap.cc> when using
> preemptible RCU:
> 
> [   48.180297] BUG: using smp_processor_id() in preemptible [00000000] code: ntpdate/3562
> [   48.180297] caller is __nf_conntrack_find+0x9b/0xeb [nf_conntrack]
> [   48.180297] Pid: 3562, comm: ntpdate Not tainted 2.6.25-rc2-mm1-testing #1
> [   48.180297]  [<c02015b9>] debug_smp_processor_id+0x99/0xb0
> [   48.180297]  [<fac643a7>] __nf_conntrack_find+0x9b/0xeb [nf_conntrack]
> 
> Tested-by: Tilman Schmidt <tilman@imap.cc>
> Tested-by: Christian Casteyde <casteyde.christian@free.fr> [Bugzilla #10097]
> 
> Signed-off-by: Patrick McHardy <kaber@trash.net>

Applied.

[NETFILTER 02/05]: xt_conntrack: fix missing boolean clamping

[Patrick McHardy]

[NETFILTER]: xt_conntrack: fix missing boolean clamping

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

---
commit bcc67d744bd9df15cb4e2d590b30ddadebd5a867
tree 8888741a4eb78658db89b968d066837478238597
parent 2ffa8857857148a5b0c0823852d6cb2551a3ae5a
author Jan Engelhardt <jengelh@computergmbh.de> Wed, 27 Feb 2008 13:23:56 +0100
committer Patrick McHardy <kaber@trash.net> Wed, 27 Feb 2008 13:23:56 +0100

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

diff --git a/net/netfilter/xt_conntrack.c b/net/netfilter/xt_conntrack.c
index 8533085..dd192ac 100644
--- a/net/netfilter/xt_conntrack.c
+++ b/net/netfilter/xt_conntrack.c
@@ -231,7 +231,7 @@ conntrack_mt(const struct sk_buff *skb, const struct net_device *in,
 			if (test_bit(IPS_DST_NAT_BIT, &ct->status))
 				statebit |= XT_CONNTRACK_STATE_DNAT;
 		}
-		if ((info->state_mask & statebit) ^
+		if (!!(info->state_mask & statebit) ^
 		    !(info->invert_flags & XT_CONNTRACK_STATE))
 			return false;
 	}

Re: [NETFILTER 02/05]: xt_conntrack: fix missing boolean clamping

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Wed, 27 Feb 2008 14:14:22 +0100 (MET)

> [NETFILTER]: xt_conntrack: fix missing boolean clamping
> 
> Signed-off-by: Jan Engelhardt <jengelh@computergmbh.de>
> Signed-off-by: Patrick McHardy <kaber@trash.net>

Applied, thanks.

[NETFILTER 03/05]: xt_conntrack: fix IPv4 address comparison

[Patrick McHardy]

[NETFILTER]: xt_conntrack: fix IPv4 address comparison

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

---
commit 54b4910d81a787d932d0a3237a8a15879f8eb8b8
tree c1551c39ec76a38a5ee9cb92af07e98ade545bbb
parent bcc67d744bd9df15cb4e2d590b30ddadebd5a867
author Jan Engelhardt <jengelh@computergmbh.de> Wed, 27 Feb 2008 13:23:57 +0100
committer Patrick McHardy <kaber@trash.net> Wed, 27 Feb 2008 13:23:57 +0100

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

diff --git a/net/netfilter/xt_conntrack.c b/net/netfilter/xt_conntrack.c
index dd192ac..0c50b28 100644
--- a/net/netfilter/xt_conntrack.c
+++ b/net/netfilter/xt_conntrack.c
@@ -122,7 +122,7 @@ conntrack_addrcmp(const union nf_inet_addr *kaddr,
                   const union nf_inet_addr *umask, unsigned int l3proto)
 {
 	if (l3proto == AF_INET)
-		return (kaddr->ip & umask->ip) == uaddr->ip;
+		return ((kaddr->ip ^ uaddr->ip) & umask->ip) == 0;
 	else if (l3proto == AF_INET6)
 		return ipv6_masked_addr_cmp(&kaddr->in6, &umask->in6,
 		       &uaddr->in6) == 0;

Re: [NETFILTER 03/05]: xt_conntrack: fix IPv4 address comparison

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Wed, 27 Feb 2008 14:14:23 +0100 (MET)

> [NETFILTER]: xt_conntrack: fix IPv4 address comparison
> 
> Signed-off-by: Jan Engelhardt <jengelh@computergmbh.de>
> Signed-off-by: Patrick McHardy <kaber@trash.net>

Applied.

[NETFILTER 04/05]: nf_nat: always select same SNAT source for same host

[Patrick McHardy]

[NETFILTER]: nf_nat: always select same SNAT source for same host

We've removed the SAME target in 2.6.25-rc since it had 32/64 bit compat
problems and the NAT core provides the same behaviour regarding IP
selection. This turned out to be not entirely correct though, the
NAT core only selects the same IP from a range for the same src,dst
combination. Some people need the same IP for all destinations however.

The easiest way to do this is to ignore the destination IP when
doing SNAT. Since we're using jhash, we still get good distribution
for multiple source IPs.

Tested-by: David Lau <mintypickle@gmail.com>

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

---
commit c31b70def66807822dc973e66757507a5fd4c3b6
tree c0a635a5e75bbce4e234ca53d3f12e9af963e741
parent 54b4910d81a787d932d0a3237a8a15879f8eb8b8
author Patrick McHardy <kaber@trash.net> Wed, 27 Feb 2008 13:23:57 +0100
committer Patrick McHardy <kaber@trash.net> Wed, 27 Feb 2008 13:23:57 +0100

 net/ipv4/netfilter/nf_nat_core.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/netfilter/nf_nat_core.c b/net/ipv4/netfilter/nf_nat_core.c
index 0d5fa3a..8e1cae2 100644
--- a/net/ipv4/netfilter/nf_nat_core.c
+++ b/net/ipv4/netfilter/nf_nat_core.c
@@ -188,15 +188,19 @@ find_best_ips_proto(struct nf_conntrack_tuple *tuple,
 	__be32 *var_ipp;
 	/* Host order */
 	u_int32_t minip, maxip, j;
+	__be32 dst;
 
 	/* No IP mapping?  Do nothing. */
 	if (!(range->flags & IP_NAT_RANGE_MAP_IPS))
 		return;
 
-	if (maniptype == IP_NAT_MANIP_SRC)
+	if (maniptype == IP_NAT_MANIP_SRC) {
 		var_ipp = &tuple->src.u3.ip;
-	else
+		dst = 0;
+	} else {
 		var_ipp = &tuple->dst.u3.ip;
+		dst = tuple->dst.u3.ip;
+	}
 
 	/* Fast path: only one choice. */
 	if (range->min_ip == range->max_ip) {
@@ -212,8 +216,7 @@ find_best_ips_proto(struct nf_conntrack_tuple *tuple,
 	 * like this), even across reboots. */
 	minip = ntohl(range->min_ip);
 	maxip = ntohl(range->max_ip);
-	j = jhash_2words((__force u32)tuple->src.u3.ip,
-			 (__force u32)tuple->dst.u3.ip, 0);
+	j = jhash_2words((__force u32)tuple->src.u3.ip, (__force u32)dst, 0);
 	j = ((u64)j * (maxip - minip + 1)) >> 32;
 	*var_ipp = htonl(minip + j);
 }

Re: [NETFILTER 04/05]: nf_nat: always select same SNAT source for same host

[Patrick McHardy]

Patrick McHardy wrote:
> [NETFILTER]: nf_nat: always select same SNAT source for same host
> 
> We've removed the SAME target in 2.6.25-rc since it had 32/64 bit compat
> problems and the NAT core provides the same behaviour regarding IP
> selection. This turned out to be not entirely correct though, the
> NAT core only selects the same IP from a range for the same src,dst
> combination. Some people need the same IP for all destinations however.
> 
> The easiest way to do this is to ignore the destination IP when
> doing SNAT. Since we're using jhash, we still get good distribution
> for multiple source IPs.
> 
> Tested-by: David Lau <mintypickle@gmail.com>
> 
> Signed-off-by: Patrick McHardy <kaber@trash.net>


Please drop this patch for now, David reported some bad distribution
during further tests that I want to look into.

Re: [NETFILTER 04/05]: nf_nat: always select same SNAT source for same host

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Wed, 27 Feb 2008 17:31:59 +0100

> Please drop this patch for now, David reported some bad distribution
> during further tests that I want to look into.

Ok.

Re: [NETFILTER 04/05]: nf_nat: always select same SNAT source for same host

[Bernhard Schmidt]

Patrick McHardy <kaber@trash.net> wrote:

Hello Patrick,

> Patrick McHardy wrote:
>> [NETFILTER]: nf_nat: always select same SNAT source for same host
>> 
>> We've removed the SAME target in 2.6.25-rc since it had 32/64 bit compat
>> problems and the NAT core provides the same behaviour regarding IP
>> selection. This turned out to be not entirely correct though, the
>> NAT core only selects the same IP from a range for the same src,dst
>> combination. Some people need the same IP for all destinations however.
>> 
>> The easiest way to do this is to ignore the destination IP when
>> doing SNAT. Since we're using jhash, we still get good distribution
>> for multiple source IPs.
>> 
>> Tested-by: David Lau <mintypickle@gmail.com>
>> 
>> Signed-off-by: Patrick McHardy <kaber@trash.net>
>
>
> Please drop this patch for now, David reported some bad distribution
> during further tests that I want to look into.

Any news on that? We're getting hit by that issue (ICQ fails to login,
amongst others). In 2.6.25, but I did not see any patch in recent
kernels that changes this.

Bernhard

[NETFILTER 05/05]: Fix NF_QUEUE_NR() parenthesis

[Patrick McHardy]

[NETFILTER]: Fix NF_QUEUE_NR() parenthesis

Properly add parens around the macro argument. This is not needed by
the kernel but the macro is exported to userspace, so it shouldn't
make any assumptions.

Also use NF_VERDICT_BITS instead of NF_VERDICT_QBTIS for the left-shift
since thats whats logically correct.

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

---
commit 81d9aa76da78b14124638e761a4c0acab3d43508
tree d9ec5a1792894c1068aaf4eaab865a5b6001c939
parent c31b70def66807822dc973e66757507a5fd4c3b6
author Patrick McHardy <kaber@trash.net> Wed, 27 Feb 2008 13:23:58 +0100
committer Patrick McHardy <kaber@trash.net> Wed, 27 Feb 2008 13:23:58 +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 b74b615..f0680c2 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -31,7 +31,7 @@
 #define NF_VERDICT_QMASK 0xffff0000
 #define NF_VERDICT_QBITS 16
 
-#define NF_QUEUE_NR(x) (((x << NF_VERDICT_QBITS) & NF_VERDICT_QMASK) | NF_QUEUE)
+#define NF_QUEUE_NR(x) ((((x) << NF_VERDICT_BITS) & NF_VERDICT_QMASK) | NF_QUEUE)
 
 /* only for userspace compatibility */
 #ifndef __KERNEL__

Re: [NETFILTER 05/05]: Fix NF_QUEUE_NR() parenthesis

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Wed, 27 Feb 2008 14:14:26 +0100 (MET)

> [NETFILTER]: Fix NF_QUEUE_NR() parenthesis
> 
> Properly add parens around the macro argument. This is not needed by
> the kernel but the macro is exported to userspace, so it shouldn't
> make any assumptions.
> 
> Also use NF_VERDICT_BITS instead of NF_VERDICT_QBTIS for the left-shift
> since thats whats logically correct.
> 
> Signed-off-by: Patrick McHardy <kaber@trash.net>

Applied, thanks.

[NETFILTER 00/05]: Netfilter fixes

[Patrick McHardy]

Hi Dave,

following are a few important netfilter fixes for 2.6.20, fixing a
REJECT target regression in 2.6.19, a nf_nat crash and an ebtables
crash. Also included are two patches to use the correct type for
iptables compat offsets and remove the EXPERIMENTAL mark from
nf_conntrack.

Please apply, thanks.


 net/bridge/netfilter/ebtables.c     |    3 ++-
 net/ipv4/netfilter.c                |    7 +++++--
 net/ipv4/netfilter/Kconfig          |    4 ++--
 net/ipv4/netfilter/ip_tables.c      |   10 +++++-----
 net/ipv4/netfilter/ipt_MASQUERADE.c |    5 ++++-
 net/netfilter/Kconfig               |   25 ++++++++++++-------------
 6 files changed, 30 insertions(+), 24 deletions(-)

Chuck Ebbert:
      [NETFILTER]: ebtables: don't compute gap before checking struct type

Dmitry Mishin:
      [NETFILTER]: compat offsets size change

Martin Josefsson:
      [NETFILTER]: nf_nat: fix MASQUERADE crash on device down

Patrick McHardy:
      [NETFILTER]: Fix routing of REJECT target generated packets in output chain
      [NETFILTER]: New connection tracking is not EXPERIMENTAL anymore

[NETFILTER 00/05]: Netfilter fixes

[Patrick McHardy]

Hi Dave,

following are a few netfilter fixes. The iptables hook validation fixes
are quite critical, so I'm going to send them to -stable along with Bart's
fix.

Please apply, thanks.


 include/linux/netfilter/nf_conntrack_pptp.h |    3 
 net/bridge/br_netfilter.c                   |   36 +++++-
 net/ipv4/netfilter/arp_tables.c             |   48 ++++-----
 net/ipv4/netfilter/ip_tables.c              |  146 ++++++++++++++--------------
 net/ipv6/netfilter/ip6_tables.c             |   59 ++++-------
 net/netfilter/nf_conntrack_expect.c         |   27 ++---
 6 files changed, 168 insertions(+), 151 deletions(-)

Bart De Schuymer:
      [NETFILTER]: bridge netfilter: deal with martians correctly

Dmitry Mishin:
      [NETFILTER]: Fix {ip,ip6,arp}_tables hook validation
      [NETFILTER]: Fix iptables compat hook validation

Yasuyuki Kozakai:
      [NETFILTER]: nf_conntrack: fix warning in PPTP helper
      [NETFILTER]: nf_conntrack: Don't try to find clashed expectation

Re: [NETFILTER 00/05]: Netfilter fixes

[David Miller]

From: Patrick McHardy <kaber@trash.net>
Date: Mon,  4 Dec 2006 11:55:56 +0100 (MET)

> following are a few netfilter fixes. The iptables hook validation fixes
> are quite critical, so I'm going to send them to -stable along with Bart's
> fix.
> 
> Please apply, thanks.

All applied, thanks Patrick.

[NETFILTER 00/05]: Netfilter fixes

[Patrick McHardy]

Hi Dave,

the following patches contain a few important iptables fixes from the OpenVZ
guys, a fix for a nf_conntrack regression from the listhelp.h removal and
a small Kconfig update. I'll pass the important ones on to -stable once
I've caught up with all the previous fixes.

Please apply, thanks.


 net/ipv4/netfilter/arp_tables.c   |   25 +++++++++-----
 net/ipv4/netfilter/ip_tables.c    |   67 +++++++++++++++++++-------------------
 net/ipv6/netfilter/Kconfig        |    2 -
 net/ipv6/netfilter/ip6_tables.c   |   24 +++++++++----
 net/netfilter/nf_conntrack_core.c |    3 +
 5 files changed, 69 insertions(+), 52 deletions(-)

Dmitry Mishin:
      [NETFILTER]: Missed and reordered checks in {arp,ip,ip6}_tables
      [NETFILTER]: ip_tables: compat code module refcounting fix

Martin Josefsson:
      [NETFILTER]: nf_conntrack: add missing unlock in get_next_corpse()

Peter Bieringer:
      [NETFILTER]: remove masq/NAT from ip6tables Kconfig help

Vasily Averin:
      [NETFILTER]: ip_tables: compat error way cleanup