Skip to content

Commit

Permalink
Merge git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf-next
Browse files Browse the repository at this point in the history 
Pablo Neira Ayuso says:

====================
Netfilter/nf_tables fixes for net-next

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.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Jun 5, 2014
2 parents 54a4ca8 + 31f8441 commit 6934e79
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 20 deletions.
59 changes: 41 additions & 18 deletions net/netfilter/nf_tables_api.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -1169,14 +1171,15 @@ 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);
err = nft_trans_chain_add(&ctx, NFT_MSG_DELCHAIN);
if (err < 0)
return err;

table->use--;
list_del(&chain->list);
return 0;
}
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -2186,7 +2191,7 @@ static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx,

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;
Expand All @@ -2197,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;

Expand All @@ -2208,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);
Expand Down Expand Up @@ -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:
Expand All @@ -2607,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);
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -3077,7 +3084,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:
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -3161,7 +3170,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)
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand All @@ -3392,24 +3401,29 @@ 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_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:
nft_trans_elem_set(trans)->nelems++;
nf_tables_setelem_notify(&trans->ctx,
nft_trans_elem_set(trans),
&nft_trans_elem(trans),
NFT_MSG_NEWSETELEM, 0);
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),
Expand Down Expand Up @@ -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) {
Expand All @@ -3496,38 +3511,46 @@ 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;
}
}

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);
Expand Down
1 change: 0 additions & 1 deletion net/netfilter/nfnetlink_acct.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
22 changes: 21 additions & 1 deletion net/netfilter/nft_rbtree.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand All @@ -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);

Expand All @@ -53,6 +56,8 @@ static bool nft_rbtree_lookup(const struct nft_set *set,
goto out;
if (set->flags & NFT_SET_MAP)
nft_data_copy(data, rbe->data);

spin_unlock_bh(&nft_rbtree_lock);
return true;
}
}
Expand All @@ -62,6 +67,7 @@ static bool nft_rbtree_lookup(const struct nft_set *set,
goto found;
}
out:
spin_unlock_bh(&nft_rbtree_lock);
return false;
}

Expand Down Expand Up @@ -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;
}

Expand All @@ -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);
}

Expand All @@ -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);

Expand All @@ -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;
}

Expand All @@ -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;
Expand All @@ -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[])
Expand All @@ -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,
Expand Down

0 comments on commit 6934e79

Please sign in to comment.