netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] netfilter: netfilter fixes
@ 2011-02-02 23:45 kaber
  2011-02-02 23:45 ` [PATCH 1/4] netfilter: ctnetlink: fix missing refcount increment during dumps kaber
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: kaber @ 2011-02-02 23:45 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev

Hi Dave,

following are a couple of netfilter fixes for 2.6.38, containing:

- a missing conntrack reference count increment during ctnetlink table
  dumping, leading to a crash, from Pablo

- a fix for mismatches in the IPv6 iprange match, from Thomas Jacob

- a fix to make arp_tables mangling work again after some cleanups,
  from Pablo

- a fix for missing information in ctnetlink when events are filtered
  using the CT target, from Pablo

Please pull from:

git://git.kernel.org/pub/scm/linux/kernel/git/kaber/nf-2.6.git master

Thanks!

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/4] netfilter: ctnetlink: fix missing refcount increment during dumps
  2011-02-02 23:45 [PATCH 0/4] netfilter: netfilter fixes kaber
@ 2011-02-02 23:45 ` kaber
  2011-02-02 23:45 ` [PATCH 2/4] netfilter: xt_iprange: Incorrect xt_iprange boundary check for IPv6 kaber
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: kaber @ 2011-02-02 23:45 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev

From: Pablo Neira Ayuso <pablo@netfilter.org>

In 13ee6ac netfilter: fix race in conntrack between dump_table and
destroy, we recovered spinlocks to protect the dump of the conntrack
table according to reports from Stephen and acknowledgments on the
issue from Eric.

In that patch, the refcount bump that allows to keep a reference
to the current ct object was removed. However, we still decrement
the refcount for that object in the output path of
ctnetlink_dump_table():

        if (last)
                nf_ct_put(last)

Cc: Stephen Hemminger <stephen.hemminger@vyatta.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 net/netfilter/nf_conntrack_netlink.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 93297aa..eead9db 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -667,6 +667,7 @@ restart:
 			if (ctnetlink_fill_info(skb, NETLINK_CB(cb->skb).pid,
 						cb->nlh->nlmsg_seq,
 						IPCTNL_MSG_CT_NEW, ct) < 0) {
+				nf_conntrack_get(&ct->ct_general);
 				cb->args[1] = (unsigned long)ct;
 				goto out;
 			}
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/4] netfilter: xt_iprange: Incorrect xt_iprange boundary check for IPv6
  2011-02-02 23:45 [PATCH 0/4] netfilter: netfilter fixes kaber
  2011-02-02 23:45 ` [PATCH 1/4] netfilter: ctnetlink: fix missing refcount increment during dumps kaber
@ 2011-02-02 23:45 ` kaber
  2011-02-02 23:45 ` [PATCH 3/4] netfilter: arpt_mangle: fix return values of checkentry kaber
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: kaber @ 2011-02-02 23:45 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev

From: Thomas Jacob <jacob@internet24.de>

iprange_ipv6_sub was substracting 2 unsigned ints and then casting
the result to int to find out whether they are lt, eq or gt each
other, this doesn't work if the full 32 bits of each part
can be used in IPv6 addresses. Patch should remedy that without
significant performance penalties. Also number of ntohl
calls can be reduced this way (Jozsef Kadlecsik).

Signed-off-by: Thomas Jacob <jacob@internet24.de>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 net/netfilter/xt_iprange.c |   16 +++++++---------
 1 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/xt_iprange.c b/net/netfilter/xt_iprange.c
index 88f7c35..73c33a4 100644
--- a/net/netfilter/xt_iprange.c
+++ b/net/netfilter/xt_iprange.c
@@ -53,15 +53,13 @@ iprange_mt4(const struct sk_buff *skb, struct xt_action_param *par)
 }
 
 static inline int
-iprange_ipv6_sub(const struct in6_addr *a, const struct in6_addr *b)
+iprange_ipv6_lt(const struct in6_addr *a, const struct in6_addr *b)
 {
 	unsigned int i;
-	int r;
 
 	for (i = 0; i < 4; ++i) {
-		r = ntohl(a->s6_addr32[i]) - ntohl(b->s6_addr32[i]);
-		if (r != 0)
-			return r;
+		if (a->s6_addr32[i] != b->s6_addr32[i])
+			return ntohl(a->s6_addr32[i]) < ntohl(b->s6_addr32[i]);
 	}
 
 	return 0;
@@ -75,15 +73,15 @@ iprange_mt6(const struct sk_buff *skb, struct xt_action_param *par)
 	bool m;
 
 	if (info->flags & IPRANGE_SRC) {
-		m  = iprange_ipv6_sub(&iph->saddr, &info->src_min.in6) < 0;
-		m |= iprange_ipv6_sub(&iph->saddr, &info->src_max.in6) > 0;
+		m  = iprange_ipv6_lt(&iph->saddr, &info->src_min.in6);
+		m |= iprange_ipv6_lt(&info->src_max.in6, &iph->saddr);
 		m ^= !!(info->flags & IPRANGE_SRC_INV);
 		if (m)
 			return false;
 	}
 	if (info->flags & IPRANGE_DST) {
-		m  = iprange_ipv6_sub(&iph->daddr, &info->dst_min.in6) < 0;
-		m |= iprange_ipv6_sub(&iph->daddr, &info->dst_max.in6) > 0;
+		m  = iprange_ipv6_lt(&iph->daddr, &info->dst_min.in6);
+		m |= iprange_ipv6_lt(&info->dst_max.in6, &iph->daddr);
 		m ^= !!(info->flags & IPRANGE_DST_INV);
 		if (m)
 			return false;
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/4] netfilter: arpt_mangle: fix return values of checkentry
  2011-02-02 23:45 [PATCH 0/4] netfilter: netfilter fixes kaber
  2011-02-02 23:45 ` [PATCH 1/4] netfilter: ctnetlink: fix missing refcount increment during dumps kaber
  2011-02-02 23:45 ` [PATCH 2/4] netfilter: xt_iprange: Incorrect xt_iprange boundary check for IPv6 kaber
@ 2011-02-02 23:45 ` kaber
  2011-02-02 23:45 ` [PATCH 4/4] netfilter: ecache: always set events bits, filter them later kaber
  2011-02-02 23:52 ` [PATCH 0/4] netfilter: netfilter fixes David Miller
  4 siblings, 0 replies; 6+ messages in thread
From: kaber @ 2011-02-02 23:45 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev

From: Pablo Neira Ayuso <pablo@netfilter.org>

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: 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>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 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 = {
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 4/4] netfilter: ecache: always set events bits, filter them later
  2011-02-02 23:45 [PATCH 0/4] netfilter: netfilter fixes kaber
                   ` (2 preceding siblings ...)
  2011-02-02 23:45 ` [PATCH 3/4] netfilter: arpt_mangle: fix return values of checkentry kaber
@ 2011-02-02 23:45 ` kaber
  2011-02-02 23:52 ` [PATCH 0/4] netfilter: netfilter fixes David Miller
  4 siblings, 0 replies; 6+ messages in thread
From: kaber @ 2011-02-02 23:45 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev

From: Pablo Neira Ayuso <pablo@netfilter.org>

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>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 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);
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/4] netfilter: netfilter fixes
  2011-02-02 23:45 [PATCH 0/4] netfilter: netfilter fixes kaber
                   ` (3 preceding siblings ...)
  2011-02-02 23:45 ` [PATCH 4/4] netfilter: ecache: always set events bits, filter them later kaber
@ 2011-02-02 23:52 ` David Miller
  4 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2011-02-02 23:52 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel, netdev

From: kaber@trash.net
Date: Thu,  3 Feb 2011 00:45:26 +0100

> following are a couple of netfilter fixes for 2.6.38, containing:
> 
> - a missing conntrack reference count increment during ctnetlink table
>   dumping, leading to a crash, from Pablo
> 
> - a fix for mismatches in the IPv6 iprange match, from Thomas Jacob
> 
> - a fix to make arp_tables mangling work again after some cleanups,
>   from Pablo
> 
> - a fix for missing information in ctnetlink when events are filtered
>   using the CT target, from Pablo
> 
> Please pull from:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/kaber/nf-2.6.git master

Pulled, thanks Patrick.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2011-02-02 23:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-02 23:45 [PATCH 0/4] netfilter: netfilter fixes kaber
2011-02-02 23:45 ` [PATCH 1/4] netfilter: ctnetlink: fix missing refcount increment during dumps kaber
2011-02-02 23:45 ` [PATCH 2/4] netfilter: xt_iprange: Incorrect xt_iprange boundary check for IPv6 kaber
2011-02-02 23:45 ` [PATCH 3/4] netfilter: arpt_mangle: fix return values of checkentry kaber
2011-02-02 23:45 ` [PATCH 4/4] netfilter: ecache: always set events bits, filter them later kaber
2011-02-02 23:52 ` [PATCH 0/4] netfilter: netfilter fixes David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).