netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] netfilter: netfilter fixes
@ 2010-05-20 16:00 kaber
  2010-05-20 16:00 ` [PATCH 1/3] netfilter: nf_ct_sip: handle non-linear skbs kaber
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: kaber @ 2010-05-20 16:00 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev

following are three fixes for netfilter:

- handling of non-linear skbs in the SIP conntrack helper. This fixes tracking
  failures when running on the same machine as a SIP application that is
  producing non-linear skbs. Long term this should be fixed by using the string
  search API, but that is a bigger piece of work.

- fix for a race condition in nf_conntrack that might lead to conntrack entries
  marked as dead entering the hash tables, blocking new connections with similar
  keys.

- a fix for an incorrect comment about the checkentry return conventions.

Please apply or pull from:

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

Thanks!

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

* [PATCH 1/3] netfilter: nf_ct_sip: handle non-linear skbs
  2010-05-20 16:00 [PATCH 0/3] netfilter: netfilter fixes kaber
@ 2010-05-20 16:00 ` kaber
  2010-05-20 16:00 ` [PATCH 2/3] netfilter: nf_conntrack: fix a race in __nf_conntrack_confirm against nf_ct_get_next_corpse() kaber
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: kaber @ 2010-05-20 16:00 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev

From: Patrick McHardy <kaber@trash.net>

Handle non-linear skbs by linearizing them instead of silently failing.
Long term the helper should be fixed to either work with non-linear skbs
directly by using the string search API or work on a copy of the data.

Based on patch by Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 net/netfilter/nf_conntrack_sip.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/nf_conntrack_sip.c b/net/netfilter/nf_conntrack_sip.c
index b20f427..53d8922 100644
--- a/net/netfilter/nf_conntrack_sip.c
+++ b/net/netfilter/nf_conntrack_sip.c
@@ -1393,10 +1393,8 @@ static int sip_help_tcp(struct sk_buff *skb, unsigned int protoff,
 
 	nf_ct_refresh(ct, skb, sip_timeout * HZ);
 
-	if (skb_is_nonlinear(skb)) {
-		pr_debug("Copy of skbuff not supported yet.\n");
-		return NF_ACCEPT;
-	}
+	if (unlikely(skb_linearize(skb)))
+		return NF_DROP;
 
 	dptr = skb->data + dataoff;
 	datalen = skb->len - dataoff;
@@ -1455,10 +1453,8 @@ static int sip_help_udp(struct sk_buff *skb, unsigned int protoff,
 
 	nf_ct_refresh(ct, skb, sip_timeout * HZ);
 
-	if (skb_is_nonlinear(skb)) {
-		pr_debug("Copy of skbuff not supported yet.\n");
-		return NF_ACCEPT;
-	}
+	if (unlikely(skb_linearize(skb)))
+		return NF_DROP;
 
 	dptr = skb->data + dataoff;
 	datalen = skb->len - dataoff;
-- 
1.7.0.4


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

* [PATCH 2/3] netfilter: nf_conntrack: fix a race in __nf_conntrack_confirm against nf_ct_get_next_corpse()
  2010-05-20 16:00 [PATCH 0/3] netfilter: netfilter fixes kaber
  2010-05-20 16:00 ` [PATCH 1/3] netfilter: nf_ct_sip: handle non-linear skbs kaber
@ 2010-05-20 16:00 ` kaber
  2010-05-20 16:00 ` [PATCH 3/3] netfilter: fix description of expected checkentry return code on xt_target kaber
  2010-05-21  6:12 ` [PATCH 0/3] netfilter: netfilter fixes David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: kaber @ 2010-05-20 16:00 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev

From: Joerg Marx <joerg.marx@secunet.com>

This race was triggered by a 'conntrack -F' command running in parallel
to the insertion of a hash for a new connection. Losing this race led to
a dead conntrack entry effectively blocking traffic for a particular
connection until timeout or flushing the conntrack hashes again.
Now the check for an already dying connection is done inside the lock.

Signed-off-by: Joerg Marx <joerg.marx@secunet.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 include/net/netfilter/nf_conntrack_core.h |    2 +-
 net/netfilter/nf_conntrack_core.c         |   10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
index dffde8e..3d7524f 100644
--- a/include/net/netfilter/nf_conntrack_core.h
+++ b/include/net/netfilter/nf_conntrack_core.h
@@ -61,7 +61,7 @@ static inline int nf_conntrack_confirm(struct sk_buff *skb)
 	int ret = NF_ACCEPT;
 
 	if (ct && ct != &nf_conntrack_untracked) {
-		if (!nf_ct_is_confirmed(ct) && !nf_ct_is_dying(ct))
+		if (!nf_ct_is_confirmed(ct))
 			ret = __nf_conntrack_confirm(skb);
 		if (likely(ret == NF_ACCEPT))
 			nf_ct_deliver_cached_events(ct);
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index b83c530..eeeb8bc 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -424,6 +424,16 @@ __nf_conntrack_confirm(struct sk_buff *skb)
 
 	spin_lock_bh(&nf_conntrack_lock);
 
+	/* We have to check the DYING flag inside the lock to prevent
+	   a race against nf_ct_get_next_corpse() possibly called from
+	   user context, else we insert an already 'dead' hash, blocking
+	   further use of that particular connection -JM */
+
+	if (unlikely(nf_ct_is_dying(ct))) {
+		spin_unlock_bh(&nf_conntrack_lock);
+		return NF_ACCEPT;
+	}
+
 	/* See if there's one in the list already, including reverse:
 	   NAT could have grabbed it without realizing, since we're
 	   not in the hash.  If there is, we lost race. */
-- 
1.7.0.4


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

* [PATCH 3/3] netfilter: fix description of expected checkentry return code on xt_target
  2010-05-20 16:00 [PATCH 0/3] netfilter: netfilter fixes kaber
  2010-05-20 16:00 ` [PATCH 1/3] netfilter: nf_ct_sip: handle non-linear skbs kaber
  2010-05-20 16:00 ` [PATCH 2/3] netfilter: nf_conntrack: fix a race in __nf_conntrack_confirm against nf_ct_get_next_corpse() kaber
@ 2010-05-20 16:00 ` kaber
  2010-05-21  6:12 ` [PATCH 0/3] netfilter: netfilter fixes David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: kaber @ 2010-05-20 16:00 UTC (permalink / raw)
  To: davem; +Cc: netfilter-devel, netdev

From: Luciano Coelho <luciano.coelho@nokia.com>

The text describing the return codes that are expected on calls to
checkentry() was incorrect.  Instead of returning true or false, or an error
code, it should return 0 or an error code.

Signed-off-by: Luciano Coelho <luciano.coelho@nokia.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
 include/linux/netfilter/x_tables.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/netfilter/x_tables.h b/include/linux/netfilter/x_tables.h
index c2ee5d8..c00cc0c 100644
--- a/include/linux/netfilter/x_tables.h
+++ b/include/linux/netfilter/x_tables.h
@@ -333,7 +333,7 @@ struct xt_target {
 	/* Called when user tries to insert an entry of this type:
            hook_mask is a bitmask of hooks from which it can be
            called. */
-	/* Should return true or false, or an error code (-Exxxx). */
+	/* Should return 0 on success or an error code otherwise (-Exxxx). */
 	int (*checkentry)(const struct xt_tgchk_param *);
 
 	/* Called when entry of this type deleted. */
-- 
1.7.0.4


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

* Re: [PATCH 0/3] netfilter: netfilter fixes
  2010-05-20 16:00 [PATCH 0/3] netfilter: netfilter fixes kaber
                   ` (2 preceding siblings ...)
  2010-05-20 16:00 ` [PATCH 3/3] netfilter: fix description of expected checkentry return code on xt_target kaber
@ 2010-05-21  6:12 ` David Miller
  3 siblings, 0 replies; 5+ messages in thread
From: David Miller @ 2010-05-21  6:12 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel, netdev

From: kaber@trash.net
Date: Thu, 20 May 2010 18:00:43 +0200

> following are three fixes for netfilter:
> 
> - handling of non-linear skbs in the SIP conntrack helper. This fixes tracking
>   failures when running on the same machine as a SIP application that is
>   producing non-linear skbs. Long term this should be fixed by using the string
>   search API, but that is a bigger piece of work.
> 
> - fix for a race condition in nf_conntrack that might lead to conntrack entries
>   marked as dead entering the hash tables, blocking new connections with similar
>   keys.
> 
> - a fix for an incorrect comment about the checkentry return conventions.
> 
> Please apply or pull from:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/kaber/nf-next-2.6.git master

Pulled, thanks Patrick.

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

end of thread, other threads:[~2010-05-21  6:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-20 16:00 [PATCH 0/3] netfilter: netfilter fixes kaber
2010-05-20 16:00 ` [PATCH 1/3] netfilter: nf_ct_sip: handle non-linear skbs kaber
2010-05-20 16:00 ` [PATCH 2/3] netfilter: nf_conntrack: fix a race in __nf_conntrack_confirm against nf_ct_get_next_corpse() kaber
2010-05-20 16:00 ` [PATCH 3/3] netfilter: fix description of expected checkentry return code on xt_target kaber
2010-05-21  6:12 ` [PATCH 0/3] 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).