All of lore.kernel.org
 help / color / mirror / Atom feed
From: Patrick McHardy <kaber@trash.net>
To: davem@davemloft.net
Cc: Patrick McHardy <kaber@trash.net>, netfilter-devel@vger.kernel.org
Subject: netfilter 03/06: ctnetlink: fix double helper assignation for NAT'ed conntracks
Date: Mon, 18 Aug 2008 18:51:52 +0200 (MEST)	[thread overview]
Message-ID: <20080818165151.18978.90816.sendpatchset@localhost.localdomain> (raw)
In-Reply-To: <20080818165147.18978.92208.sendpatchset@localhost.localdomain>

netfilter: ctnetlink: fix double helper assignation for NAT'ed conntracks

If we create a conntrack that has NAT handlings and a helper, the helper
is assigned twice. This happens because nf_nat_setup_info() - via
nf_conntrack_alter_reply() - sets the helper before ctnetlink, which
indeed does not check if the conntrack already has a helper as it thinks that
it is a brand new conntrack.

The fix moves the helper assignation before the set of the status flags.
This avoids a bogus assertion in __nf_ct_ext_add (if netfilter assertions are
enabled) which checks that the conntrack must not be confirmed.

This problem was introduced in 2.6.23 with the netfilter extension
infrastructure.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 4a58ff2a369c6020ece596812f819641f9d19820
tree 7c640e1ffb18d61e9ef3a097e2b2ab00eb529c52
parent 132fbda27191848d809ac4c4e5841581371aafd8
author Pablo Neira Ayuso <pablo@netfilter.org> Mon, 18 Aug 2008 13:17:03 +0200
committer Patrick McHardy <kaber@trash.net> Mon, 18 Aug 2008 13:17:03 +0200

 net/netfilter/nf_conntrack_netlink.c |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 105a616..d1fb2f8 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -1136,16 +1136,33 @@ ctnetlink_create_conntrack(struct nlattr *cda[],
 	ct->timeout.expires = jiffies + ct->timeout.expires * HZ;
 	ct->status |= IPS_CONFIRMED;
 
+	rcu_read_lock();
+	helper = __nf_ct_helper_find(rtuple);
+	if (helper) {
+		help = nf_ct_helper_ext_add(ct, GFP_KERNEL);
+		if (help == NULL) {
+			rcu_read_unlock();
+			err = -ENOMEM;
+			goto err;
+		}
+		/* not in hash table yet so not strictly necessary */
+		rcu_assign_pointer(help->helper, helper);
+	}
+
 	if (cda[CTA_STATUS]) {
 		err = ctnetlink_change_status(ct, cda);
-		if (err < 0)
+		if (err < 0) {
+			rcu_read_unlock();
 			goto err;
+		}
 	}
 
 	if (cda[CTA_PROTOINFO]) {
 		err = ctnetlink_change_protoinfo(ct, cda);
-		if (err < 0)
+		if (err < 0) {
+			rcu_read_unlock();
 			goto err;
+		}
 	}
 
 	nf_ct_acct_ext_add(ct, GFP_KERNEL);
@@ -1155,19 +1172,6 @@ ctnetlink_create_conntrack(struct nlattr *cda[],
 		ct->mark = ntohl(nla_get_be32(cda[CTA_MARK]));
 #endif
 
-	rcu_read_lock();
-	helper = __nf_ct_helper_find(rtuple);
-	if (helper) {
-		help = nf_ct_helper_ext_add(ct, GFP_KERNEL);
-		if (help == NULL) {
-			rcu_read_unlock();
-			err = -ENOMEM;
-			goto err;
-		}
-		/* not in hash table yet so not strictly necessary */
-		rcu_assign_pointer(help->helper, helper);
-	}
-
 	/* setup master conntrack: this is a confirmed expectation */
 	if (master_ct) {
 		__set_bit(IPS_EXPECTED_BIT, &ct->status);

  parent reply	other threads:[~2008-08-18 16:51 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-18 16:51 netfilter 00/06: netfilter fixes Patrick McHardy
2008-08-18 16:51 ` netfilter 01/06: Move linux/in.h and linux/in6.h inclusions outside of Patrick McHardy
2008-08-18 16:57   ` Jan Engelhardt
2008-08-18 17:01     ` Patrick McHardy
2008-08-19  4:21       ` David Miller
2008-08-18 16:51 ` netfilter 02/06: ipt_addrtype: fix matching of inverted destination address type Patrick McHardy
2008-08-19  4:30   ` David Miller
2008-08-18 16:51 ` Patrick McHardy [this message]
2008-08-19  4:31   ` netfilter 03/06: ctnetlink: fix double helper assignation for NAT'ed conntracks David Miller
2008-08-18 16:51 ` netfilter 04/06: ctnetlink: fix sleep in read-side lock section Patrick McHardy
2008-08-19  4:31   ` David Miller
2008-08-18 16:51 ` netfilter 05/06: ctnetlink: sleepable allocation with spin lock bh Patrick McHardy
2008-08-19  4:31   ` David Miller
2008-08-18 16:51 ` nf_nat 06/06: use secure_ipv4_port_ephemeral() for NAT port randomization Patrick McHardy
2008-08-19  4:32   ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080818165151.18978.90816.sendpatchset@localhost.localdomain \
    --to=kaber@trash.net \
    --cc=davem@davemloft.net \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.