[PATCH 0/2] bugfixes for nf-next-2.6 (2.6.38-rc)

[PATCH 0/2] bugfixes for nf-next-2.6 (2.6.38-rc)

[Pablo Neira Ayuso]

Hi Patrick!

I've got a couple of patches for nf-next-2.6. The first fixes
arptables that seems to be broken since quite some time due
to a broken cleanup. The latter refers to the CT event filtering,
I think it can be qualified as bugfix since the current filtering
schema is not useful for conntrackd.

Please, apply!

---

Pablo Neira Ayuso (2):
      netfilter: arpt_mangle: fix return values of checkentry
      netfilter: ecache: always set events bits, filter them later


 include/net/netfilter/nf_conntrack_ecache.h |    3 ---
 net/ipv4/netfilter/arpt_mangle.c            |    6 +++---
 net/netfilter/nf_conntrack_ecache.c         |    3 +++
 3 files changed, 6 insertions(+), 6 deletions(-)

--
1+1=1

[PATCH 1/2] netfilter: arpt_mangle: fix return values of checkentry

[Pablo Neira Ayuso]

In 135367b "netfilter: xtables: change xt_target.checkentry return type",
the type returned by checkentry was changed from boolean to int, but the
return values where not adjusted.

# arptables -I INPUT -i eth1 --h-length 6 --destination-mac 01:00:5e:00:01:01 -j mangle --mangle-mac-d 00:18:71:68:f2:34
arptables: Input/output error

This broke arptables with the mangle target since it returns true
under success, which is interpreted by xtables as >0, thus
returning EIO.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv4/netfilter/arpt_mangle.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/netfilter/arpt_mangle.c b/net/ipv4/netfilter/arpt_mangle.c
index b8ddcc4..a5e52a9 100644
--- a/net/ipv4/netfilter/arpt_mangle.c
+++ b/net/ipv4/netfilter/arpt_mangle.c
@@ -60,12 +60,12 @@ static int checkentry(const struct xt_tgchk_param *par)
 
 	if (mangle->flags & ~ARPT_MANGLE_MASK ||
 	    !(mangle->flags & ARPT_MANGLE_MASK))
-		return false;
+		return -EINVAL;
 
 	if (mangle->target != NF_DROP && mangle->target != NF_ACCEPT &&
 	   mangle->target != XT_CONTINUE)
-		return false;
-	return true;
+		return -EINVAL;
+	return 0;
 }
 
 static struct xt_target arpt_mangle_reg __read_mostly = {

[PATCH 2/2] netfilter: ecache: always set events bits, filter them later

[Pablo Neira Ayuso]

For the following rule:

iptables -I PREROUTING -t raw -j CT --ctevents assured

The event delivered looks like the following:

 [UPDATE] tcp      6 src=192.168.0.2 dst=192.168.1.2 sport=37041 dport=80 src=192.168.1.2 dst=192.168.1.100 sport=80 dport=37041 [ASSURED]

Note that the TCP protocol state is not included. For that reason
the CT event filtering is not very useful for conntrackd.

To resolve this issue, instead of conditionally setting the CT events
bits based on the ctmask, we always set them and perform the filtering
in the late stage, just before the delivery.

Thus, the event delivered looks like the following:

 [UPDATE] tcp      6 432000 ESTABLISHED src=192.168.0.2 dst=192.168.1.2 sport=37041 dport=80 src=192.168.1.2 dst=192.168.1.100 sport=80 dport=37041 [ASSURED]

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_conntrack_ecache.h |    3 ---
 net/netfilter/nf_conntrack_ecache.c         |    3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_ecache.h b/include/net/netfilter/nf_conntrack_ecache.h
index 96ba5f7..349cefe 100644
--- a/include/net/netfilter/nf_conntrack_ecache.h
+++ b/include/net/netfilter/nf_conntrack_ecache.h
@@ -77,9 +77,6 @@ nf_conntrack_event_cache(enum ip_conntrack_events event, struct nf_conn *ct)
 	if (e == NULL)
 		return;
 
-	if (!(e->ctmask & (1 << event)))
-		return;
-
 	set_bit(event, &e->cache);
 }
 
diff --git a/net/netfilter/nf_conntrack_ecache.c b/net/netfilter/nf_conntrack_ecache.c
index 5702de3..63a1b91 100644
--- a/net/netfilter/nf_conntrack_ecache.c
+++ b/net/netfilter/nf_conntrack_ecache.c
@@ -63,6 +63,9 @@ void nf_ct_deliver_cached_events(struct nf_conn *ct)
 		 * this does not harm and it happens very rarely. */
 		unsigned long missed = e->missed;
 
+		if (!((events | missed) & e->ctmask))
+			goto out_unlock;
+
 		ret = notify->fcn(events | missed, &item);
 		if (unlikely(ret < 0 || missed)) {
 			spin_lock_bh(&ct->lock);

Re: [PATCH 0/2] bugfixes for nf-next-2.6 (2.6.38-rc)

[Patrick McHardy]

Am 01.02.2011 13:05, schrieb Pablo Neira Ayuso:
> Hi Patrick!
> 
> I've got a couple of patches for nf-next-2.6. The first fixes
> arptables that seems to be broken since quite some time due
> to a broken cleanup. The latter refers to the CT event filtering,
> I think it can be qualified as bugfix since the current filtering
> schema is not useful for conntrackd.

The subject is contradictional, nf-next-2.6/net-next-2.6 will be
merged once 2.6.38 is released, not for 2.6.38-rc. I suppose these
fixes should go into nf-2.6.git for 2.6.38-rc?

Re: [PATCH 0/2] bugfixes for nf-next-2.6 (2.6.38-rc)

[Pablo Neira Ayuso]

On 01/02/11 13:45, Patrick McHardy wrote:
> Am 01.02.2011 13:05, schrieb Pablo Neira Ayuso:
>> Hi Patrick!
>>
>> I've got a couple of patches for nf-next-2.6. The first fixes
>> arptables that seems to be broken since quite some time due
>> to a broken cleanup. The latter refers to the CT event filtering,
>> I think it can be qualified as bugfix since the current filtering
>> schema is not useful for conntrackd.
> 
> The subject is contradictional, nf-next-2.6/net-next-2.6 will be
> merged once 2.6.38 is released, not for 2.6.38-rc. I suppose these
> fixes should go into nf-2.6.git for 2.6.38-rc?

Indeed, I think this should go to nf-2.6.git for 2.6.38-rc. Sorry!

Re: [PATCH 1/2] netfilter: arpt_mangle: fix return values of checkentry

[Patrick McHardy]

Am 01.02.2011 13:05, schrieb Pablo Neira Ayuso:
> In 135367b "netfilter: xtables: change xt_target.checkentry return type",
> the type returned by checkentry was changed from boolean to int, but the
> return values where not adjusted.
> 
> # arptables -I INPUT -i eth1 --h-length 6 --destination-mac 01:00:5e:00:01:01 -j mangle --mangle-mac-d 00:18:71:68:f2:34
> arptables: Input/output error
> 
> This broke arptables with the mangle target since it returns true
> under success, which is interpreted by xtables as >0, thus
> returning EIO.

Applied, thanks Pablo. I'll also push this to -stable once it hits
Linus' tree.

Re: [PATCH 2/2] netfilter: ecache: always set events bits, filter them later

[Patrick McHardy]

Am 01.02.2011 13:05, schrieb Pablo Neira Ayuso:
> For the following rule:
> 
> iptables -I PREROUTING -t raw -j CT --ctevents assured
> 
> The event delivered looks like the following:
> 
>  [UPDATE] tcp      6 src=192.168.0.2 dst=192.168.1.2 sport=37041 dport=80 src=192.168.1.2 dst=192.168.1.100 sport=80 dport=37041 [ASSURED]
> 
> Note that the TCP protocol state is not included. For that reason
> the CT event filtering is not very useful for conntrackd.
> 
> To resolve this issue, instead of conditionally setting the CT events
> bits based on the ctmask, we always set them and perform the filtering
> in the late stage, just before the delivery.
> 
> Thus, the event delivered looks like the following:
> 
>  [UPDATE] tcp      6 432000 ESTABLISHED src=192.168.0.2 dst=192.168.1.2 sport=37041 dport=80 src=192.168.1.2 dst=192.168.1.100 sport=80 dport=37041 [ASSURED]
> 

Looks good to me, applied, thanks. Do you want me to push this
one to -stable as well?