Commit 71977437 authored by Florian Westphal's avatar Florian Westphal Committed by Pablo Neira Ayuso
Browse files

netfilter: conntrack: convert to refcount_t api 



Convert nf_conn reference counting from atomic_t to refcount_t based api.
refcount_t api provides more runtime sanity checks and will warn on
certain constructs, e.g. refcount_inc() on a zero reference count, which
usually indicates use-after-free.

For this reason template allocation is changed to init the refcount to
1, the subsequenct add operations are removed.

Likewise, init_conntrack() is changed to set the initial refcount to 1
instead refcount_inc().

This is safe because the new entry is not (yet) visible to other cpus.

Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent 613a0c67

include/linux/netfilter/nf_conntrack_common.h

+4 −4
Original line number Diff line number Diff line
@@ -2,7 +2,7 @@ 
#ifndef _NF_CONNTRACK_COMMON_H

#define _NF_CONNTRACK_COMMON_H



#include <linux/atomic.h>

#include <linux/refcount.h>

#include <uapi/linux/netfilter/nf_conntrack_common.h>



struct ip_conntrack_stat {
@@ -25,19 +25,19 @@ struct ip_conntrack_stat { 
#define NFCT_PTRMASK	~(NFCT_INFOMASK)



struct nf_conntrack {

	atomic_t use;

	refcount_t use;

};



void nf_conntrack_destroy(struct nf_conntrack *nfct);

static inline void nf_conntrack_put(struct nf_conntrack *nfct)

{

	if (nfct && atomic_dec_and_test(&nfct->use))

	if (nfct && refcount_dec_and_test(&nfct->use))

		nf_conntrack_destroy(nfct);

}

static inline void nf_conntrack_get(struct nf_conntrack *nfct)

{

	if (nfct)

		atomic_inc(&nfct->use);

		refcount_inc(&nfct->use);

}



#endif /* _NF_CONNTRACK_COMMON_H */

net/netfilter/nf_conntrack_core.c

+13 −13
Original line number Diff line number Diff line
@@ -585,7 +585,7 @@ struct nf_conn *nf_ct_tmpl_alloc(struct net *net, 
	tmpl->status = IPS_TEMPLATE;

	write_pnet(&tmpl->ct_net, net);

	nf_ct_zone_add(tmpl, zone);

	atomic_set(&tmpl->ct_general.use, 0);

	refcount_set(&tmpl->ct_general.use, 1);



	return tmpl;

}
@@ -618,7 +618,7 @@ destroy_conntrack(struct nf_conntrack *nfct) 
	struct nf_conn *ct = (struct nf_conn *)nfct;



	pr_debug("destroy_conntrack(%p)\n", ct);

	WARN_ON(atomic_read(&nfct->use) != 0);

	WARN_ON(refcount_read(&nfct->use) != 0);



	if (unlikely(nf_ct_is_template(ct))) {

		nf_ct_tmpl_free(ct);
@@ -742,7 +742,7 @@ nf_ct_match(const struct nf_conn *ct1, const struct nf_conn *ct2) 
/* caller must hold rcu readlock and none of the nf_conntrack_locks */

static void nf_ct_gc_expired(struct nf_conn *ct)

{

	if (!atomic_inc_not_zero(&ct->ct_general.use))

	if (!refcount_inc_not_zero(&ct->ct_general.use))

		return;



	if (nf_ct_should_gc(ct))
@@ -810,7 +810,7 @@ __nf_conntrack_find_get(struct net *net, const struct nf_conntrack_zone *zone, 
		 * in, try to obtain a reference and re-check tuple

		 */

		ct = nf_ct_tuplehash_to_ctrack(h);

		if (likely(atomic_inc_not_zero(&ct->ct_general.use))) {

		if (likely(refcount_inc_not_zero(&ct->ct_general.use))) {

			if (likely(nf_ct_key_equal(h, tuple, zone, net)))

				goto found;
@@ -907,7 +907,7 @@ nf_conntrack_hash_check_insert(struct nf_conn *ct) 


	smp_wmb();

	/* The caller holds a reference to this object */

	atomic_set(&ct->ct_general.use, 2);

	refcount_set(&ct->ct_general.use, 2);

	__nf_conntrack_hash_insert(ct, hash, reply_hash);

	nf_conntrack_double_unlock(hash, reply_hash);

	NF_CT_STAT_INC(net, insert);
@@ -958,7 +958,7 @@ static void __nf_conntrack_insert_prepare(struct nf_conn *ct) 
{

	struct nf_conn_tstamp *tstamp;



	atomic_inc(&ct->ct_general.use);

	refcount_inc(&ct->ct_general.use);

	ct->status |= IPS_CONFIRMED;



	/* set conntrack timestamp, if enabled. */
@@ -1351,7 +1351,7 @@ static unsigned int early_drop_list(struct net *net, 
		    nf_ct_is_dying(tmp))

			continue;



		if (!atomic_inc_not_zero(&tmp->ct_general.use))

		if (!refcount_inc_not_zero(&tmp->ct_general.use))

			continue;



		/* kill only if still in same netns -- might have moved due to
@@ -1469,7 +1469,7 @@ static void gc_worker(struct work_struct *work) 
				continue;



			/* need to take reference to avoid possible races */

			if (!atomic_inc_not_zero(&tmp->ct_general.use))

			if (!refcount_inc_not_zero(&tmp->ct_general.use))

				continue;



			if (gc_worker_skip_ct(tmp)) {
@@ -1569,7 +1569,7 @@ __nf_conntrack_alloc(struct net *net, 
	/* Because we use RCU lookups, we set ct_general.use to zero before

	 * this is inserted in any list.

	 */

	atomic_set(&ct->ct_general.use, 0);

	refcount_set(&ct->ct_general.use, 0);

	return ct;

out:

	atomic_dec(&cnet->count);
@@ -1594,7 +1594,7 @@ void nf_conntrack_free(struct nf_conn *ct) 
	/* A freed object has refcnt == 0, that's

	 * the golden rule for SLAB_TYPESAFE_BY_RCU

	 */

	WARN_ON(atomic_read(&ct->ct_general.use) != 0);

	WARN_ON(refcount_read(&ct->ct_general.use) != 0);



	nf_ct_ext_destroy(ct);

	kmem_cache_free(nf_conntrack_cachep, ct);
@@ -1686,8 +1686,8 @@ init_conntrack(struct net *net, struct nf_conn *tmpl, 
	if (!exp)

		__nf_ct_try_assign_helper(ct, tmpl, GFP_ATOMIC);



	/* Now it is inserted into the unconfirmed list, bump refcount */

	nf_conntrack_get(&ct->ct_general);

	/* Now it is inserted into the unconfirmed list, set refcount to 1. */

	refcount_set(&ct->ct_general.use, 1);

	nf_ct_add_to_unconfirmed_list(ct);



	local_bh_enable();
@@ -2300,7 +2300,7 @@ get_next_corpse(int (*iter)(struct nf_conn *i, void *data), 


	return NULL;

found:

	atomic_inc(&ct->ct_general.use);

	refcount_inc(&ct->ct_general.use);

	spin_unlock(lockp);

	local_bh_enable();

	return ct;

net/netfilter/nf_conntrack_expect.c

+2 −2
Original line number Diff line number Diff line
@@ -203,12 +203,12 @@ nf_ct_find_expectation(struct net *net, 
	 * about to invoke ->destroy(), or nf_ct_delete() via timeout

	 * or early_drop().

	 *

	 * The atomic_inc_not_zero() check tells:  If that fails, we

	 * The refcount_inc_not_zero() check tells:  If that fails, we

	 * know that the ct is being destroyed.  If it succeeds, we

	 * can be sure the ct cannot disappear underneath.

	 */

	if (unlikely(nf_ct_is_dying(exp->master) ||

		     !atomic_inc_not_zero(&exp->master->ct_general.use)))

		     !refcount_inc_not_zero(&exp->master->ct_general.use)))

		return NULL;



	if (exp->flags & NF_CT_EXPECT_PERMANENT) {

net/netfilter/nf_conntrack_netlink.c

+3 −3
Original line number Diff line number Diff line
@@ -508,7 +508,7 @@ static int ctnetlink_dump_id(struct sk_buff *skb, const struct nf_conn *ct) 


static int ctnetlink_dump_use(struct sk_buff *skb, const struct nf_conn *ct)

{

	if (nla_put_be32(skb, CTA_USE, htonl(atomic_read(&ct->ct_general.use))))

	if (nla_put_be32(skb, CTA_USE, htonl(refcount_read(&ct->ct_general.use))))

		goto nla_put_failure;

	return 0;
@@ -1200,7 +1200,7 @@ ctnetlink_dump_table(struct sk_buff *skb, struct netlink_callback *cb) 
			ct = nf_ct_tuplehash_to_ctrack(h);

			if (nf_ct_is_expired(ct)) {

				if (i < ARRAY_SIZE(nf_ct_evict) &&

				    atomic_inc_not_zero(&ct->ct_general.use))

				    refcount_inc_not_zero(&ct->ct_general.use))

					nf_ct_evict[i++] = ct;

				continue;

			}
@@ -1748,7 +1748,7 @@ ctnetlink_dump_list(struct sk_buff *skb, struct netlink_callback *cb, bool dying 
						  NFNL_MSG_TYPE(cb->nlh->nlmsg_type),

						  ct, dying, 0);

			if (res < 0) {

				if (!atomic_inc_not_zero(&ct->ct_general.use))

				if (!refcount_inc_not_zero(&ct->ct_general.use))

					continue;

				cb->args[0] = cpu;

				cb->args[1] = (unsigned long)ct;

net/netfilter/nf_conntrack_standalone.c

+2 −2
Original line number Diff line number Diff line
@@ -303,7 +303,7 @@ static int ct_seq_show(struct seq_file *s, void *v) 
	int ret = 0;



	WARN_ON(!ct);

	if (unlikely(!atomic_inc_not_zero(&ct->ct_general.use)))

	if (unlikely(!refcount_inc_not_zero(&ct->ct_general.use)))

		return 0;



	if (nf_ct_should_gc(ct)) {
@@ -370,7 +370,7 @@ static int ct_seq_show(struct seq_file *s, void *v) 
	ct_show_zone(s, ct, NF_CT_DEFAULT_ZONE_DIR);

	ct_show_delta_time(s, ct);



	seq_printf(s, "use=%u\n", atomic_read(&ct->ct_general.use));

	seq_printf(s, "use=%u\n", refcount_read(&ct->ct_general.use));



	if (seq_has_overflowed(s))

		goto release;