netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: netfilter-devel@vger.kernel.org
Cc: davem@davemloft.net, netdev@vger.kernel.org
Subject: [PATCH 13/35] netfilter: conntrack: fix crash on timeout object removal
Date: Sat, 17 Oct 2015 13:32:48 +0200	[thread overview]
Message-ID: <1445081590-2924-14-git-send-email-pablo@netfilter.org> (raw)
In-Reply-To: <1445081590-2924-1-git-send-email-pablo@netfilter.org>

The object and module refcounts are updated for each conntrack template,
however, if we delete the iptables rules and we flush the timeout
database, we may end up with invalid references to timeout object that
are just gone.

Resolve this problem by setting the timeout reference to NULL when the
custom timeout entry is removed from our base. This patch requires some
RCU trickery to ensure safe pointer handling.

This handling is similar to what we already do with conntrack helpers,
the idea is to avoid bumping the timeout object reference counter from
the packet path to avoid the cost of atomic ops.

Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_conntrack_timeout.h | 25 ++++++++++++++++-----
 net/netfilter/nf_conntrack_core.c            | 12 ++++++----
 net/netfilter/nfnetlink_cttimeout.c          | 33 ++++++++++++++++++++++++++++
 net/netfilter/xt_CT.c                        |  4 +++-
 4 files changed, 63 insertions(+), 11 deletions(-)

diff --git a/include/net/netfilter/nf_conntrack_timeout.h b/include/net/netfilter/nf_conntrack_timeout.h
index 6230871..f72be38 100644
--- a/include/net/netfilter/nf_conntrack_timeout.h
+++ b/include/net/netfilter/nf_conntrack_timeout.h
@@ -20,10 +20,20 @@ struct ctnl_timeout {
 };
 
 struct nf_conn_timeout {
-	struct ctnl_timeout	*timeout;
+	struct ctnl_timeout __rcu *timeout;
 };
 
-#define NF_CT_TIMEOUT_EXT_DATA(__t) (unsigned int *) &((__t)->timeout->data)
+static inline unsigned int *
+nf_ct_timeout_data(struct nf_conn_timeout *t)
+{
+	struct ctnl_timeout *timeout;
+
+	timeout = rcu_dereference(t->timeout);
+	if (timeout == NULL)
+		return NULL;
+
+	return (unsigned int *)timeout->data;
+}
 
 static inline
 struct nf_conn_timeout *nf_ct_timeout_find(const struct nf_conn *ct)
@@ -47,7 +57,7 @@ struct nf_conn_timeout *nf_ct_timeout_ext_add(struct nf_conn *ct,
 	if (timeout_ext == NULL)
 		return NULL;
 
-	timeout_ext->timeout = timeout;
+	rcu_assign_pointer(timeout_ext->timeout, timeout);
 
 	return timeout_ext;
 #else
@@ -64,10 +74,13 @@ nf_ct_timeout_lookup(struct net *net, struct nf_conn *ct,
 	unsigned int *timeouts;
 
 	timeout_ext = nf_ct_timeout_find(ct);
-	if (timeout_ext)
-		timeouts = NF_CT_TIMEOUT_EXT_DATA(timeout_ext);
-	else
+	if (timeout_ext) {
+		timeouts = nf_ct_timeout_data(timeout_ext);
+		if (unlikely(!timeouts))
+			timeouts = l4proto->get_timeouts(net);
+	} else {
 		timeouts = l4proto->get_timeouts(net);
+	}
 
 	return timeouts;
 #else
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 09d1d19..3cb3cb8 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -940,10 +940,13 @@ init_conntrack(struct net *net, struct nf_conn *tmpl,
 	}
 
 	timeout_ext = tmpl ? nf_ct_timeout_find(tmpl) : NULL;
-	if (timeout_ext)
-		timeouts = NF_CT_TIMEOUT_EXT_DATA(timeout_ext);
-	else
+	if (timeout_ext) {
+		timeouts = nf_ct_timeout_data(timeout_ext);
+		if (unlikely(!timeouts))
+			timeouts = l4proto->get_timeouts(net);
+	} else {
 		timeouts = l4proto->get_timeouts(net);
+	}
 
 	if (!l4proto->new(ct, skb, dataoff, timeouts)) {
 		nf_conntrack_free(ct);
@@ -952,7 +955,8 @@ init_conntrack(struct net *net, struct nf_conn *tmpl,
 	}
 
 	if (timeout_ext)
-		nf_ct_timeout_ext_add(ct, timeout_ext->timeout, GFP_ATOMIC);
+		nf_ct_timeout_ext_add(ct, rcu_dereference(timeout_ext->timeout),
+				      GFP_ATOMIC);
 
 	nf_ct_acct_ext_add(ct, GFP_ATOMIC);
 	nf_ct_tstamp_ext_add(ct, GFP_ATOMIC);
diff --git a/net/netfilter/nfnetlink_cttimeout.c b/net/netfilter/nfnetlink_cttimeout.c
index 476accd..5bda647 100644
--- a/net/netfilter/nfnetlink_cttimeout.c
+++ b/net/netfilter/nfnetlink_cttimeout.c
@@ -291,6 +291,34 @@ cttimeout_get_timeout(struct sock *ctnl, struct sk_buff *skb,
 	return ret;
 }
 
+static void untimeout(struct nf_conntrack_tuple_hash *i,
+		      struct ctnl_timeout *timeout)
+{
+	struct nf_conn *ct = nf_ct_tuplehash_to_ctrack(i);
+	struct nf_conn_timeout *timeout_ext = nf_ct_timeout_find(ct);
+
+	if (timeout_ext && (!timeout || timeout_ext->timeout == timeout))
+		RCU_INIT_POINTER(timeout_ext->timeout, NULL);
+}
+
+static void ctnl_untimeout(struct ctnl_timeout *timeout)
+{
+	struct nf_conntrack_tuple_hash *h;
+	const struct hlist_nulls_node *nn;
+	int i;
+
+	local_bh_disable();
+	for (i = 0; i < init_net.ct.htable_size; i++) {
+		spin_lock(&nf_conntrack_locks[i % CONNTRACK_LOCKS]);
+		if (i < init_net.ct.htable_size) {
+			hlist_nulls_for_each_entry(h, nn, &init_net.ct.hash[i], hnnode)
+				untimeout(h, timeout);
+		}
+		spin_unlock(&nf_conntrack_locks[i % CONNTRACK_LOCKS]);
+	}
+	local_bh_enable();
+}
+
 /* try to delete object, fail if it is still in use. */
 static int ctnl_timeout_try_del(struct ctnl_timeout *timeout)
 {
@@ -301,6 +329,7 @@ static int ctnl_timeout_try_del(struct ctnl_timeout *timeout)
 		/* We are protected by nfnl mutex. */
 		list_del_rcu(&timeout->head);
 		nf_ct_l4proto_put(timeout->l4proto);
+		ctnl_untimeout(timeout);
 		kfree_rcu(timeout, rcu_head);
 	} else {
 		/* still in use, restore reference counter. */
@@ -567,6 +596,10 @@ static void __exit cttimeout_exit(void)
 	pr_info("cttimeout: unregistering from nfnetlink.\n");
 
 	nfnetlink_subsys_unregister(&cttimeout_subsys);
+
+	/* Make sure no conntrack objects refer to custom timeouts anymore. */
+	ctnl_untimeout(NULL);
+
 	list_for_each_entry_safe(cur, tmp, &cttimeout_list, head) {
 		list_del_rcu(&cur->head);
 		/* We are sure that our objects have no clients at this point,
diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
index a03924c..e7ac07e 100644
--- a/net/netfilter/xt_CT.c
+++ b/net/netfilter/xt_CT.c
@@ -321,8 +321,10 @@ static void xt_ct_destroy_timeout(struct nf_conn *ct)
 
 	if (timeout_put) {
 		timeout_ext = nf_ct_timeout_find(ct);
-		if (timeout_ext)
+		if (timeout_ext) {
 			timeout_put(timeout_ext->timeout);
+			RCU_INIT_POINTER(timeout_ext->timeout, NULL);
+		}
 	}
 	rcu_read_unlock();
 #endif
-- 
2.1.4


  parent reply	other threads:[~2015-10-17 11:26 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-17 11:32 [PATCH 00/35] Netfilter/IPVS updates for net-next Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 01/35] ipvs: Don't protect ip_vs_addr_is_unicast with CONFIG_SYSCTL Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 02/35] netfilter: nfnetlink_queue: get rid of nfnetlink_queue_ct.c Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 03/35] netfilter: rename nfnetlink_queue_core.c to nfnetlink_queue.c Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 04/35] netfilter: nfnetlink_queue: use y2038 safe timestamp Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 05/35] netfilter: remove dead code Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 06/35] netfilter: nfnetlink_queue: rename related to nfqueue attaching conntrack info Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 07/35] netfilter: Kconfig rename QUEUE_CT to GLUE_CT Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 08/35] netfilter: ctnetlink: add const qualifier to nfnl_hook.get_ct Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 09/35] netfilter: nfnetlink_log: allow to attach conntrack Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 10/35] ipvs: Remove possibly unused variable from ip_vs_out Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 11/35] ipvs: Remove possibly unused variables from ip_vs_conn_net_{init,cleanup} Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 12/35] netfilter: xt_CT: don't put back reference to timeout policy object Pablo Neira Ayuso
2015-10-17 11:32 ` Pablo Neira Ayuso [this message]
2015-10-17 11:32 ` [PATCH 14/35] nfnetlink_cttimeout: add rcu_barrier() on module removal Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 15/35] netfilter: bridge: avoid unused label warning Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 16/35] netfilter: nfnetlink_log: autoload nf_conntrack_netlink module NFQA_CFG_F_CONNTRACK config flag Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 17/35] netfilter: nfqueue: don't use prev pointer Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 18/35] netfilter: ip6_tables: label placement Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 19/35] netfilter: ip6_tables: function definition layout Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 20/35] netfilter: ipv6: code indentation Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 21/35] netfilter: ipv6: whitespace around operators Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 22/35] netfilter: ip6_tables: ternary operator layout Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 23/35] netfilter: ip6_tables: improve if statements Pablo Neira Ayuso
2015-10-17 11:32 ` [PATCH 24/35] netfilter: ipv6: pointer cast layout Pablo Neira Ayuso
2015-10-17 11:33 ` [PATCH 25/35] netfilter: nfnetlink_log: consolidate check for instance in nfulnl_recv_config() Pablo Neira Ayuso
2015-10-17 11:33 ` [PATCH 26/35] netfilter: nfnetlink_log: validate dependencies to avoid breaking atomicity Pablo Neira Ayuso
2015-10-17 11:33 ` [PATCH 27/35] netfilter: remove hook owner refcounting Pablo Neira Ayuso
2015-10-17 11:33 ` [PATCH 28/35] netfilter: make nf_queue_entry_get_refs return void Pablo Neira Ayuso
2015-10-17 11:33 ` [PATCH 29/35] netfilter: nf_queue: remove rcu_read_lock calls Pablo Neira Ayuso
2015-10-17 11:33 ` [PATCH 30/35] netfilter: turn NF_HOOK into an inline function Pablo Neira Ayuso
2015-10-17 11:33 ` [PATCH 31/35] netfilter: ipv4: label placement Pablo Neira Ayuso
2015-10-17 11:33 ` [PATCH 32/35] netfilter: ipv4: ternary operator layout Pablo Neira Ayuso
2015-10-17 11:33 ` [PATCH 33/35] netfilter: ipv4: function definition layout Pablo Neira Ayuso
2015-10-17 11:33 ` [PATCH 34/35] netfilter: ipv4: code indentation Pablo Neira Ayuso
2015-10-17 11:33 ` [PATCH 35/35] netfilter: ipv4: whitespace around operators Pablo Neira Ayuso
2015-10-17 13:07 ` [PATCH 00/35] Netfilter/IPVS updates for net-next David Miller
2015-10-17 13:38   ` Pablo Neira Ayuso

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=1445081590-2924-14-git-send-email-pablo@netfilter.org \
    --to=pablo@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --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 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).