netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] Netfilter/nf_tables fixes for net-next
@ 2014-06-05 15:08 Pablo Neira Ayuso
  2014-06-05 15:08 ` [PATCH 1/6] netfilter: nfnetlink_acct: Fix memory leak Pablo Neira Ayuso
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2014-06-05 15:08 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Hi David,

This patchset contains fixes for recent updates available in your
net-next, they are:

1) Fix double memory allocation for accounting objects that results
   in a leak, this slipped through with the new quota extension,
   patch from Mathieu Poirier.

2) Fix broken ordering when adding set element transactions.

3) Make sure that objects are released in reverse order in the abort
   path, to avoid possible use-after-free when accessing dependencies.

4) Allow to delete several objects (as long as dependencies are
   fulfilled) by using one batch. This includes changes in the use
   counter semantics of the nf_tables objects.

5) Fix illegal sleeping allocation from rcu callback.

You can pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next.git

Thanks!

----------------------------------------------------------------

The following changes since commit 96b2e73c5471542cb9c622c4360716684f8797ed:

  Revert "net/mlx4_en: Use affinity hint" (2014-06-02 00:18:48 -0700)

are available in the git repository at:

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

for you to fetch changes up to 31f8441c328b1c038c3e44bec740fb29393a56ad:

  netfilter: nf_tables: atomic allocation in set notifications from rcu callback (2014-06-02 10:54:38 +0200)

----------------------------------------------------------------
Mathieu Poirier (1):
      netfilter: nfnetlink_acct: Fix memory leak

Pablo Neira Ayuso (5):
      netfilter: nf_tables: fix wrong transaction ordering in set elements
      netfilter: nf_tables: release objects in reverse order in the abort path
      netfilter: nft_rbtree: introduce locking
      netfilter: nf_tables: allow to delete several objects from a batch
      netfilter: nf_tables: atomic allocation in set notifications from rcu callback

 net/netfilter/nf_tables_api.c  |   59 ++++++++++++++++++++++++++++------------
 net/netfilter/nfnetlink_acct.c |    1 -
 net/netfilter/nft_rbtree.c     |   22 ++++++++++++++-
 3 files changed, 62 insertions(+), 20 deletions(-)

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

* [PATCH 1/6] netfilter: nfnetlink_acct: Fix memory leak
  2014-06-05 15:08 [PATCH 0/6] Netfilter/nf_tables fixes for net-next Pablo Neira Ayuso
@ 2014-06-05 15:08 ` Pablo Neira Ayuso
  2014-06-05 15:08 ` [PATCH 2/6] netfilter: nf_tables: fix wrong transaction ordering in set elements Pablo Neira Ayuso
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2014-06-05 15:08 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

From: Mathieu Poirier <mathieu.poirier@linaro.org>

Allocation of memory need only to happen once, that is
after the proper checks on the NFACCT_FLAGS have been
done.  Otherwise the code can return without freeing
already allocated memory.

Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nfnetlink_acct.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
index 70e86bb..54af985 100644
--- a/net/netfilter/nfnetlink_acct.c
+++ b/net/netfilter/nfnetlink_acct.c
@@ -83,7 +83,6 @@ nfnl_acct_new(struct sock *nfnl, struct sk_buff *skb,
 		return -EBUSY;
 	}
 
-	nfacct = kzalloc(sizeof(struct nf_acct), GFP_KERNEL);
 	if (tb[NFACCT_FLAGS]) {
 		flags = ntohl(nla_get_be32(tb[NFACCT_FLAGS]));
 		if (flags & ~NFACCT_F_QUOTA)
-- 
1.7.10.4

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

* [PATCH 2/6] netfilter: nf_tables: fix wrong transaction ordering in set elements
  2014-06-05 15:08 [PATCH 0/6] Netfilter/nf_tables fixes for net-next Pablo Neira Ayuso
  2014-06-05 15:08 ` [PATCH 1/6] netfilter: nfnetlink_acct: Fix memory leak Pablo Neira Ayuso
@ 2014-06-05 15:08 ` Pablo Neira Ayuso
  2014-06-05 15:08 ` [PATCH 3/6] netfilter: nf_tables: release objects in reverse order in the abort path Pablo Neira Ayuso
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2014-06-05 15:08 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

The transaction needs to be placed at the end of the commit list,
otherwise event notifications are reordered and we may crash when
releasing object via call_rcu.

This problem was introduced in 60319eb ("netfilter: nf_tables: use new
transaction infrastructure to handle elements").

Reported-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 0478847..9365531 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -3077,7 +3077,7 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
 		goto err4;
 
 	nft_trans_elem(trans) = elem;
-	list_add(&trans->list, &ctx->net->nft.commit_list);
+	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
 	return 0;
 
 err4:
@@ -3161,7 +3161,7 @@ static int nft_del_setelem(struct nft_ctx *ctx, struct nft_set *set,
 		goto err2;
 
 	nft_trans_elem(trans) = elem;
-	list_add(&trans->list, &ctx->net->nft.commit_list);
+	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
 
 	nft_data_uninit(&elem.key, NFT_DATA_VALUE);
 	if (set->flags & NFT_SET_MAP)
-- 
1.7.10.4

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

* [PATCH 3/6] netfilter: nf_tables: release objects in reverse order in the abort path
  2014-06-05 15:08 [PATCH 0/6] Netfilter/nf_tables fixes for net-next Pablo Neira Ayuso
  2014-06-05 15:08 ` [PATCH 1/6] netfilter: nfnetlink_acct: Fix memory leak Pablo Neira Ayuso
  2014-06-05 15:08 ` [PATCH 2/6] netfilter: nf_tables: fix wrong transaction ordering in set elements Pablo Neira Ayuso
@ 2014-06-05 15:08 ` Pablo Neira Ayuso
  2014-06-05 15:08 ` [PATCH 4/6] netfilter: nft_rbtree: introduce locking Pablo Neira Ayuso
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2014-06-05 15:08 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

The patch c7c32e7 ("netfilter: nf_tables: defer all object release via
rcu") indicates that we always release deleted objects in the reverse
order, but that is only needed in the abort path. These are the two
possible scenarios when releasing objects:

1) Deletion scenario in the commit path: no need to release objects in
the reverse order since userspace already ensures that dependencies are
fulfilled), ie. userspace tells us to delete rule -> ... -> rule ->
chain -> table. In this case, we have to release the objects in the
*same order* as userspace provided.

2) Deletion scenario in the abort path: we have to iterate in the reverse
order to undo what it cannot be added, ie. userspace sent us a batch
that includes: table -> chain -> rule -> ... -> rule, and that needs to
be partially undone. In this case, we have to release objects in the
reverse order to ensure that the set and chain objects point to valid
rule and table objects.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 9365531..4fffa36 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -3527,7 +3527,8 @@ static int nf_tables_abort(struct sk_buff *skb)
 		}
 	}
 
-	list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
+	list_for_each_entry_safe_reverse(trans, next,
+					 &net->nft.commit_list, list) {
 		list_del(&trans->list);
 		trans->ctx.nla = NULL;
 		call_rcu(&trans->rcu_head, nf_tables_abort_release_rcu);
-- 
1.7.10.4

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

* [PATCH 4/6] netfilter: nft_rbtree: introduce locking
  2014-06-05 15:08 [PATCH 0/6] Netfilter/nf_tables fixes for net-next Pablo Neira Ayuso
                   ` (2 preceding siblings ...)
  2014-06-05 15:08 ` [PATCH 3/6] netfilter: nf_tables: release objects in reverse order in the abort path Pablo Neira Ayuso
@ 2014-06-05 15:08 ` Pablo Neira Ayuso
  2014-06-05 15:08 ` [PATCH 5/6] netfilter: nf_tables: allow to delete several objects from a batch Pablo Neira Ayuso
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2014-06-05 15:08 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

There's no rbtree rcu version yet, so let's fall back on the spinlock
to protect the concurrent access of this structure both from user
(to update the set content) and kernel-space (in the packet path).

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nft_rbtree.c |   22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nft_rbtree.c b/net/netfilter/nft_rbtree.c
index 072e611..e1836ff 100644
--- a/net/netfilter/nft_rbtree.c
+++ b/net/netfilter/nft_rbtree.c
@@ -18,6 +18,8 @@
 #include <linux/netfilter/nf_tables.h>
 #include <net/netfilter/nf_tables.h>
 
+static DEFINE_SPINLOCK(nft_rbtree_lock);
+
 struct nft_rbtree {
 	struct rb_root		root;
 };
@@ -38,6 +40,7 @@ static bool nft_rbtree_lookup(const struct nft_set *set,
 	const struct rb_node *parent = priv->root.rb_node;
 	int d;
 
+	spin_lock_bh(&nft_rbtree_lock);
 	while (parent != NULL) {
 		rbe = rb_entry(parent, struct nft_rbtree_elem, node);
 
@@ -53,6 +56,8 @@ found:
 				goto out;
 			if (set->flags & NFT_SET_MAP)
 				nft_data_copy(data, rbe->data);
+
+			spin_unlock_bh(&nft_rbtree_lock);
 			return true;
 		}
 	}
@@ -62,6 +67,7 @@ found:
 		goto found;
 	}
 out:
+	spin_unlock_bh(&nft_rbtree_lock);
 	return false;
 }
 
@@ -124,9 +130,12 @@ static int nft_rbtree_insert(const struct nft_set *set,
 	    !(rbe->flags & NFT_SET_ELEM_INTERVAL_END))
 		nft_data_copy(rbe->data, &elem->data);
 
+	spin_lock_bh(&nft_rbtree_lock);
 	err = __nft_rbtree_insert(set, rbe);
 	if (err < 0)
 		kfree(rbe);
+
+	spin_unlock_bh(&nft_rbtree_lock);
 	return err;
 }
 
@@ -136,7 +145,9 @@ static void nft_rbtree_remove(const struct nft_set *set,
 	struct nft_rbtree *priv = nft_set_priv(set);
 	struct nft_rbtree_elem *rbe = elem->cookie;
 
+	spin_lock_bh(&nft_rbtree_lock);
 	rb_erase(&rbe->node, &priv->root);
+	spin_unlock_bh(&nft_rbtree_lock);
 	kfree(rbe);
 }
 
@@ -147,6 +158,7 @@ static int nft_rbtree_get(const struct nft_set *set, struct nft_set_elem *elem)
 	struct nft_rbtree_elem *rbe;
 	int d;
 
+	spin_lock_bh(&nft_rbtree_lock);
 	while (parent != NULL) {
 		rbe = rb_entry(parent, struct nft_rbtree_elem, node);
 
@@ -161,9 +173,11 @@ static int nft_rbtree_get(const struct nft_set *set, struct nft_set_elem *elem)
 			    !(rbe->flags & NFT_SET_ELEM_INTERVAL_END))
 				nft_data_copy(&elem->data, rbe->data);
 			elem->flags = rbe->flags;
+			spin_unlock_bh(&nft_rbtree_lock);
 			return 0;
 		}
 	}
+	spin_unlock_bh(&nft_rbtree_lock);
 	return -ENOENT;
 }
 
@@ -176,6 +190,7 @@ static void nft_rbtree_walk(const struct nft_ctx *ctx,
 	struct nft_set_elem elem;
 	struct rb_node *node;
 
+	spin_lock_bh(&nft_rbtree_lock);
 	for (node = rb_first(&priv->root); node != NULL; node = rb_next(node)) {
 		if (iter->count < iter->skip)
 			goto cont;
@@ -188,11 +203,14 @@ static void nft_rbtree_walk(const struct nft_ctx *ctx,
 		elem.flags = rbe->flags;
 
 		iter->err = iter->fn(ctx, set, iter, &elem);
-		if (iter->err < 0)
+		if (iter->err < 0) {
+			spin_unlock_bh(&nft_rbtree_lock);
 			return;
+		}
 cont:
 		iter->count++;
 	}
+	spin_unlock_bh(&nft_rbtree_lock);
 }
 
 static unsigned int nft_rbtree_privsize(const struct nlattr * const nla[])
@@ -216,11 +234,13 @@ static void nft_rbtree_destroy(const struct nft_set *set)
 	struct nft_rbtree_elem *rbe;
 	struct rb_node *node;
 
+	spin_lock_bh(&nft_rbtree_lock);
 	while ((node = priv->root.rb_node) != NULL) {
 		rb_erase(node, &priv->root);
 		rbe = rb_entry(node, struct nft_rbtree_elem, node);
 		nft_rbtree_elem_destroy(set, rbe);
 	}
+	spin_unlock_bh(&nft_rbtree_lock);
 }
 
 static bool nft_rbtree_estimate(const struct nft_set_desc *desc, u32 features,
-- 
1.7.10.4


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

* [PATCH 5/6] netfilter: nf_tables: allow to delete several objects from a batch
  2014-06-05 15:08 [PATCH 0/6] Netfilter/nf_tables fixes for net-next Pablo Neira Ayuso
                   ` (3 preceding siblings ...)
  2014-06-05 15:08 ` [PATCH 4/6] netfilter: nft_rbtree: introduce locking Pablo Neira Ayuso
@ 2014-06-05 15:08 ` Pablo Neira Ayuso
  2014-06-05 15:08 ` [PATCH 6/6] netfilter: nf_tables: atomic allocation in set notifications from rcu callback Pablo Neira Ayuso
  2014-06-05 22:35 ` [PATCH 0/6] Netfilter/nf_tables fixes for net-next David Miller
  6 siblings, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2014-06-05 15:08 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Three changes to allow the deletion of several objects with dependencies
in one transaction, they are:

1) Introduce speculative counter increment/decrement that is undone in
   the abort path if required, thus we avoid hitting -EBUSY when deleting
   the chain. The counter updates are reverted in the abort path.

2) Increment/decrement table/chain use counter for each set/rule. We need
   this to fully rely on the use counters instead of the list content,
   eg. !list_empty(&chain->rules) which evaluate true in the middle of the
   transaction.

3) Decrement table use counter when an anonymous set is bound to the
   rule in the commit path. This avoids hitting -EBUSY when deleting
   the table that contains anonymous sets. The anonymous sets are released
   in the nf_tables_rule_destroy path. This should not be a problem since
   the rule already bumped the use counter of the chain, so the bound
   anonymous set reflects dependencies through the rule object, which
   already increases the chain use counter.

So the general assumption after this patch is that the use counters are
bumped by direct object dependencies.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c |   40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 4fffa36..dbf8236 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -538,8 +538,7 @@ static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
 		return PTR_ERR(table);
 	if (table->flags & NFT_TABLE_INACTIVE)
 		return -ENOENT;
-
-	if (!list_empty(&table->chains) || !list_empty(&table->sets))
+	if (table->use > 0)
 		return -EBUSY;
 
 	nft_ctx_init(&ctx, skb, nlh, afi, table, NULL, nla);
@@ -553,6 +552,8 @@ static int nf_tables_deltable(struct sock *nlsk, struct sk_buff *skb,
 
 static void nf_tables_table_destroy(struct nft_ctx *ctx)
 {
+	BUG_ON(ctx->table->use > 0);
+
 	kfree(ctx->table);
 	module_put(ctx->afi->owner);
 }
@@ -1128,6 +1129,7 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
 	if (err < 0)
 		goto err2;
 
+	table->use++;
 	list_add_tail(&chain->list, &table->chains);
 	return 0;
 err2:
@@ -1169,7 +1171,7 @@ static int nf_tables_delchain(struct sock *nlsk, struct sk_buff *skb,
 		return PTR_ERR(chain);
 	if (chain->flags & NFT_CHAIN_INACTIVE)
 		return -ENOENT;
-	if (!list_empty(&chain->rules) || chain->use > 0)
+	if (chain->use > 0)
 		return -EBUSY;
 
 	nft_ctx_init(&ctx, skb, nlh, afi, table, chain, nla);
@@ -1177,6 +1179,7 @@ static int nf_tables_delchain(struct sock *nlsk, struct sk_buff *skb,
 	if (err < 0)
 		return err;
 
+	table->use--;
 	list_del(&chain->list);
 	return 0;
 }
@@ -1814,6 +1817,7 @@ static int nf_tables_newrule(struct sock *nlsk, struct sk_buff *skb,
 		err = -ENOMEM;
 		goto err3;
 	}
+	chain->use++;
 	return 0;
 
 err3:
@@ -1841,6 +1845,7 @@ nf_tables_delrule_one(struct nft_ctx *ctx, struct nft_rule *rule)
 		if (nft_trans_rule_add(ctx, NFT_MSG_DELRULE, rule) == NULL)
 			return -ENOMEM;
 		nft_rule_disactivate_next(ctx->net, rule);
+		ctx->chain->use--;
 		return 0;
 	}
 	return -ENOENT;
@@ -2588,6 +2593,7 @@ static int nf_tables_newset(struct sock *nlsk, struct sk_buff *skb,
 		goto err2;
 
 	list_add_tail(&set->list, &table->sets);
+	table->use++;
 	return 0;
 
 err2:
@@ -2642,6 +2648,7 @@ static int nf_tables_delset(struct sock *nlsk, struct sk_buff *skb,
 		return err;
 
 	list_del(&set->list);
+	ctx.table->use--;
 	return 0;
 }
 
@@ -3122,6 +3129,8 @@ static int nf_tables_newsetelem(struct sock *nlsk, struct sk_buff *skb,
 		err = nft_add_set_elem(&ctx, set, attr);
 		if (err < 0)
 			break;
+
+		set->nelems++;
 	}
 	return err;
 }
@@ -3196,6 +3205,8 @@ static int nf_tables_delsetelem(struct sock *nlsk, struct sk_buff *skb,
 		err = nft_del_setelem(&ctx, set, attr);
 		if (err < 0)
 			break;
+
+		set->nelems--;
 	}
 	return err;
 }
@@ -3361,15 +3372,13 @@ static int nf_tables_commit(struct sk_buff *skb)
 		case NFT_MSG_NEWCHAIN:
 			if (nft_trans_chain_update(trans))
 				nft_chain_commit_update(trans);
-			else {
+			else
 				trans->ctx.chain->flags &= ~NFT_CHAIN_INACTIVE;
-				trans->ctx.table->use++;
-			}
+
 			nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN);
 			nft_trans_destroy(trans);
 			break;
 		case NFT_MSG_DELCHAIN:
-			trans->ctx.table->use--;
 			nf_tables_chain_notify(&trans->ctx, NFT_MSG_DELCHAIN);
 			if (!(trans->ctx.table->flags & NFT_TABLE_F_DORMANT) &&
 			    trans->ctx.chain->flags & NFT_BASE_CHAIN) {
@@ -3392,6 +3401,13 @@ static int nf_tables_commit(struct sk_buff *skb)
 			break;
 		case NFT_MSG_NEWSET:
 			nft_trans_set(trans)->flags &= ~NFT_SET_INACTIVE;
+			/* This avoids hitting -EBUSY when deleting the table
+			 * from the transaction.
+			 */
+			if (nft_trans_set(trans)->flags & NFT_SET_ANONYMOUS &&
+			    !list_empty(&nft_trans_set(trans)->bindings))
+				trans->ctx.table->use--;
+
 			nf_tables_set_notify(&trans->ctx, nft_trans_set(trans),
 					     NFT_MSG_NEWSET);
 			nft_trans_destroy(trans);
@@ -3401,7 +3417,6 @@ static int nf_tables_commit(struct sk_buff *skb)
 					     NFT_MSG_DELSET);
 			break;
 		case NFT_MSG_NEWSETELEM:
-			nft_trans_elem_set(trans)->nelems++;
 			nf_tables_setelem_notify(&trans->ctx,
 						 nft_trans_elem_set(trans),
 						 &nft_trans_elem(trans),
@@ -3409,7 +3424,6 @@ static int nf_tables_commit(struct sk_buff *skb)
 			nft_trans_destroy(trans);
 			break;
 		case NFT_MSG_DELSETELEM:
-			nft_trans_elem_set(trans)->nelems--;
 			nf_tables_setelem_notify(&trans->ctx,
 						 nft_trans_elem_set(trans),
 						 &nft_trans_elem(trans),
@@ -3487,6 +3501,7 @@ static int nf_tables_abort(struct sk_buff *skb)
 
 				nft_trans_destroy(trans);
 			} else {
+				trans->ctx.table->use--;
 				list_del(&trans->ctx.chain->list);
 				if (!(trans->ctx.table->flags & NFT_TABLE_F_DORMANT) &&
 				    trans->ctx.chain->flags & NFT_BASE_CHAIN) {
@@ -3496,32 +3511,39 @@ static int nf_tables_abort(struct sk_buff *skb)
 			}
 			break;
 		case NFT_MSG_DELCHAIN:
+			trans->ctx.table->use++;
 			list_add_tail(&trans->ctx.chain->list,
 				      &trans->ctx.table->chains);
 			nft_trans_destroy(trans);
 			break;
 		case NFT_MSG_NEWRULE:
+			trans->ctx.chain->use--;
 			list_del_rcu(&nft_trans_rule(trans)->list);
 			break;
 		case NFT_MSG_DELRULE:
+			trans->ctx.chain->use++;
 			nft_rule_clear(trans->ctx.net, nft_trans_rule(trans));
 			nft_trans_destroy(trans);
 			break;
 		case NFT_MSG_NEWSET:
+			trans->ctx.table->use--;
 			list_del(&nft_trans_set(trans)->list);
 			break;
 		case NFT_MSG_DELSET:
+			trans->ctx.table->use++;
 			list_add_tail(&nft_trans_set(trans)->list,
 				      &trans->ctx.table->sets);
 			nft_trans_destroy(trans);
 			break;
 		case NFT_MSG_NEWSETELEM:
+			nft_trans_elem_set(trans)->nelems--;
 			set = nft_trans_elem_set(trans);
 			set->ops->get(set, &nft_trans_elem(trans));
 			set->ops->remove(set, &nft_trans_elem(trans));
 			nft_trans_destroy(trans);
 			break;
 		case NFT_MSG_DELSETELEM:
+			nft_trans_elem_set(trans)->nelems++;
 			nft_trans_destroy(trans);
 			break;
 		}
-- 
1.7.10.4

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

* [PATCH 6/6] netfilter: nf_tables: atomic allocation in set notifications from rcu callback
  2014-06-05 15:08 [PATCH 0/6] Netfilter/nf_tables fixes for net-next Pablo Neira Ayuso
                   ` (4 preceding siblings ...)
  2014-06-05 15:08 ` [PATCH 5/6] netfilter: nf_tables: allow to delete several objects from a batch Pablo Neira Ayuso
@ 2014-06-05 15:08 ` Pablo Neira Ayuso
  2014-06-05 22:35 ` [PATCH 0/6] Netfilter/nf_tables fixes for net-next David Miller
  6 siblings, 0 replies; 8+ messages in thread
From: Pablo Neira Ayuso @ 2014-06-05 15:08 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Use GFP_ATOMIC allocations when sending removal notifications of
anonymous sets from rcu callback context. Sleeping in that context
is illegal.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nf_tables_api.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index dbf8236..624e083 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2191,7 +2191,7 @@ nla_put_failure:
 
 static int nf_tables_set_notify(const struct nft_ctx *ctx,
 				const struct nft_set *set,
-				int event)
+				int event, gfp_t gfp_flags)
 {
 	struct sk_buff *skb;
 	u32 portid = ctx->portid;
@@ -2202,7 +2202,7 @@ static int nf_tables_set_notify(const struct nft_ctx *ctx,
 		return 0;
 
 	err = -ENOBUFS;
-	skb = nlmsg_new(NLMSG_GOODSIZE, GFP_KERNEL);
+	skb = nlmsg_new(NLMSG_GOODSIZE, gfp_flags);
 	if (skb == NULL)
 		goto err;
 
@@ -2213,7 +2213,7 @@ static int nf_tables_set_notify(const struct nft_ctx *ctx,
 	}
 
 	err = nfnetlink_send(skb, ctx->net, portid, NFNLGRP_NFTABLES,
-			     ctx->report, GFP_KERNEL);
+			     ctx->report, gfp_flags);
 err:
 	if (err < 0)
 		nfnetlink_set_err(ctx->net, portid, NFNLGRP_NFTABLES, err);
@@ -2613,7 +2613,7 @@ static void nft_set_destroy(struct nft_set *set)
 static void nf_tables_set_destroy(const struct nft_ctx *ctx, struct nft_set *set)
 {
 	list_del(&set->list);
-	nf_tables_set_notify(ctx, set, NFT_MSG_DELSET);
+	nf_tables_set_notify(ctx, set, NFT_MSG_DELSET, GFP_ATOMIC);
 	nft_set_destroy(set);
 }
 
@@ -3409,12 +3409,12 @@ static int nf_tables_commit(struct sk_buff *skb)
 				trans->ctx.table->use--;
 
 			nf_tables_set_notify(&trans->ctx, nft_trans_set(trans),
-					     NFT_MSG_NEWSET);
+					     NFT_MSG_NEWSET, GFP_KERNEL);
 			nft_trans_destroy(trans);
 			break;
 		case NFT_MSG_DELSET:
 			nf_tables_set_notify(&trans->ctx, nft_trans_set(trans),
-					     NFT_MSG_DELSET);
+					     NFT_MSG_DELSET, GFP_KERNEL);
 			break;
 		case NFT_MSG_NEWSETELEM:
 			nf_tables_setelem_notify(&trans->ctx,
-- 
1.7.10.4


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

* Re: [PATCH 0/6] Netfilter/nf_tables fixes for net-next
  2014-06-05 15:08 [PATCH 0/6] Netfilter/nf_tables fixes for net-next Pablo Neira Ayuso
                   ` (5 preceding siblings ...)
  2014-06-05 15:08 ` [PATCH 6/6] netfilter: nf_tables: atomic allocation in set notifications from rcu callback Pablo Neira Ayuso
@ 2014-06-05 22:35 ` David Miller
  6 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2014-06-05 22:35 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev

From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Thu,  5 Jun 2014 17:08:20 +0200

> This patchset contains fixes for recent updates available in your
> net-next, they are:
> 
> 1) Fix double memory allocation for accounting objects that results
>    in a leak, this slipped through with the new quota extension,
>    patch from Mathieu Poirier.
> 
> 2) Fix broken ordering when adding set element transactions.
> 
> 3) Make sure that objects are released in reverse order in the abort
>    path, to avoid possible use-after-free when accessing dependencies.
> 
> 4) Allow to delete several objects (as long as dependencies are
>    fulfilled) by using one batch. This includes changes in the use
>    counter semantics of the nf_tables objects.
> 
> 5) Fix illegal sleeping allocation from rcu callback.

Pulled, thanks a lot Pablo.

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

end of thread, other threads:[~2014-06-05 22:35 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-05 15:08 [PATCH 0/6] Netfilter/nf_tables fixes for net-next Pablo Neira Ayuso
2014-06-05 15:08 ` [PATCH 1/6] netfilter: nfnetlink_acct: Fix memory leak Pablo Neira Ayuso
2014-06-05 15:08 ` [PATCH 2/6] netfilter: nf_tables: fix wrong transaction ordering in set elements Pablo Neira Ayuso
2014-06-05 15:08 ` [PATCH 3/6] netfilter: nf_tables: release objects in reverse order in the abort path Pablo Neira Ayuso
2014-06-05 15:08 ` [PATCH 4/6] netfilter: nft_rbtree: introduce locking Pablo Neira Ayuso
2014-06-05 15:08 ` [PATCH 5/6] netfilter: nf_tables: allow to delete several objects from a batch Pablo Neira Ayuso
2014-06-05 15:08 ` [PATCH 6/6] netfilter: nf_tables: atomic allocation in set notifications from rcu callback Pablo Neira Ayuso
2014-06-05 22:35 ` [PATCH 0/6] Netfilter/nf_tables fixes for net-next 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).