netdev.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 06/29] netfilter: conntrack: don't call iter for non-confirmed conntracks
Date: Fri, 30 Jun 2017 00:53:04 +0200	[thread overview]
Message-ID: <1498776807-11124-7-git-send-email-pablo@netfilter.org> (raw)
In-Reply-To: <1498776807-11124-1-git-send-email-pablo@netfilter.org>

From: Florian Westphal <fw@strlen.de>

nf_ct_iterate_cleanup_net currently calls iter() callback also for
conntracks on the unconfirmed list, but this is unsafe.

Acesses to nf_conn are fine, but some users access the extension area
in the iter() callback, but that does only work reliably for confirmed
conntracks (ct->ext can be reallocated at any time for unconfirmed
conntrack).

The seond issue is that there is a short window where a conntrack entry
is neither on the list nor in the table: To confirm an entry, it is first
removed from the unconfirmed list, then insert into the table.

Fix this by iterating the unconfirmed list first and marking all entries
as dying, then wait for rcu grace period.

This makes sure all entries that were about to be confirmed either are
in the main table, or will be dropped soon.

Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_conntrack_core.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 2730f9df33b7..08733685d732 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1592,7 +1592,6 @@ get_next_corpse(struct net *net, int (*iter)(struct nf_conn *i, void *data),
 	struct nf_conntrack_tuple_hash *h;
 	struct nf_conn *ct;
 	struct hlist_nulls_node *n;
-	int cpu;
 	spinlock_t *lockp;
 
 	for (; *bucket < nf_conntrack_htable_size; (*bucket)++) {
@@ -1614,24 +1613,40 @@ get_next_corpse(struct net *net, int (*iter)(struct nf_conn *i, void *data),
 		cond_resched();
 	}
 
+	return NULL;
+found:
+	atomic_inc(&ct->ct_general.use);
+	spin_unlock(lockp);
+	local_bh_enable();
+	return ct;
+}
+
+static void
+__nf_ct_unconfirmed_destroy(struct net *net)
+{
+	int cpu;
+
 	for_each_possible_cpu(cpu) {
-		struct ct_pcpu *pcpu = per_cpu_ptr(net->ct.pcpu_lists, cpu);
+		struct nf_conntrack_tuple_hash *h;
+		struct hlist_nulls_node *n;
+		struct ct_pcpu *pcpu;
+
+		pcpu = per_cpu_ptr(net->ct.pcpu_lists, cpu);
 
 		spin_lock_bh(&pcpu->lock);
 		hlist_nulls_for_each_entry(h, n, &pcpu->unconfirmed, hnnode) {
+			struct nf_conn *ct;
+
 			ct = nf_ct_tuplehash_to_ctrack(h);
-			if (iter(ct, data))
-				set_bit(IPS_DYING_BIT, &ct->status);
+
+			/* we cannot call iter() on unconfirmed list, the
+			 * owning cpu can reallocate ct->ext at any time.
+			 */
+			set_bit(IPS_DYING_BIT, &ct->status);
 		}
 		spin_unlock_bh(&pcpu->lock);
 		cond_resched();
 	}
-	return NULL;
-found:
-	atomic_inc(&ct->ct_general.use);
-	spin_unlock(lockp);
-	local_bh_enable();
-	return ct;
 }
 
 void nf_ct_iterate_cleanup_net(struct net *net,
@@ -1646,6 +1661,10 @@ void nf_ct_iterate_cleanup_net(struct net *net,
 	if (atomic_read(&net->ct.count) == 0)
 		return;
 
+	__nf_ct_unconfirmed_destroy(net);
+
+	synchronize_net();
+
 	while ((ct = get_next_corpse(net, iter, data, &bucket)) != NULL) {
 		/* Time to push up daises... */
 
-- 
2.1.4

  parent reply	other threads:[~2017-06-29 22:53 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-29 22:52 [PATCH 00/29] Netfilter updates for net-next Pablo Neira Ayuso
2017-06-29 22:52 ` [PATCH 01/29] netfilter: ctnetlink: delete extra spaces Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 02/29] netfilter: ipt_CLUSTERIP: switch to nf_register_net_hook Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 03/29] netfilter: dup: resolve warnings about missing prototypes Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 04/29] netfilter: nft_rt: make local functions static Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 05/29] netfilter: conntrack: rename nf_ct_iterate_cleanup Pablo Neira Ayuso
2017-06-29 22:53 ` Pablo Neira Ayuso [this message]
2017-06-29 22:53 ` [PATCH 07/29] netfilter: conntrack: add nf_ct_iterate_destroy Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 08/29] netfilter: conntrack: restart iteration on resize Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 09/29] netfilter: nat: destroy nat mappings on module exit path only Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 10/29] netfilter: nft_set_hash: unnecessary forward declaration Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 11/29] netfilter: nf_tables: no size estimation if number of set elements is unknown Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 12/29] netfilter: nft_set_hash: use nft_rhash prefix for resizable set backend Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 13/29] netfilter: nf_tables: select set backend flavour depending on description Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 14/29] netfilter: nf_tables: pass set description to ->privsize Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 15/29] netfilter: nft_set_hash: add nft_hash_buckets() Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 16/29] netfilter: nf_tables: allow large allocations for new sets Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 17/29] netfilter: nft_set_hash: add non-resizable hashtable implementation Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 18/29] netfilter: nft_set_hash: add lookup variant for fixed size hashtable Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 19/29] netfilter: nf_ct_helper: use nf_ct_iterate_destroy to unlink helper objs Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 20/29] netfilter: cttimeout: use nf_ct_iterate_cleanup_net to unlink timeout objs Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 21/29] netfilter: ipt_CLUSTERIP: do not hold dev Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 22/29] netfilter: move table iteration out of netns exit paths Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 23/29] netns: add and use net_ns_barrier Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 24/29] netfilter: ebt: Use new helper ebt_invalid_target to check target Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 25/29] netfilter, kbuild: use canonical method to specify objs Pablo Neira Ayuso
2017-06-30 11:17   ` David Laight
2017-06-29 22:53 ` [PATCH 26/29] netfilter: use nf_conntrack_helpers_register when possible Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 27/29] netfilter: conntrack: use NFPROTO_MAX to size array Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 28/29] netfilter: nf_tables: reduce chain type table size Pablo Neira Ayuso
2017-06-29 22:53 ` [PATCH 29/29] netfilter: nfnetlink: extended ACK reporting Pablo Neira Ayuso
2017-06-30 16:09 ` [PATCH 00/29] Netfilter updates for net-next 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=1498776807-11124-7-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).