Commit b2e3d68d authored by Florian Westphal's avatar Florian Westphal Committed by Pablo Neira Ayuso

netfilter: nft_compat: destroy function must not have side effects

The nft_compat destroy function deletes the nft_xt object from a list.
This isn't allowed anymore. Destroy functions are called asynchronously,
i.e. next batch can find the object that has a pending ->destroy()
invocation:

cpu0                       cpu1
 worker
   ->destroy               for_each_entry()
	                     if (x == ...
			        return x->ops;
     list_del(x)
     kfree_rcu(x)
                           expr->ops->... // ops was free'd

To resolve this, the list_del needs to occur before the transaction
mutex gets released.  nf_tables has a 'deactivate' hook for this
purpose, so use that to unlink the object from the list.

Fixes: 0935d558 ("netfilter: nf_tables: asynchronous release")
Reported-by: default avatarTaehee Yoo <ap420073@gmail.com>
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent cf52572e
...@@ -29,6 +29,9 @@ struct nft_xt { ...@@ -29,6 +29,9 @@ struct nft_xt {
struct nft_expr_ops ops; struct nft_expr_ops ops;
refcount_t refcnt; refcount_t refcnt;
/* used only when transaction mutex is locked */
unsigned int listcnt;
/* Unlike other expressions, ops doesn't have static storage duration. /* Unlike other expressions, ops doesn't have static storage duration.
* nft core assumes they do. We use kfree_rcu so that nft core can * nft core assumes they do. We use kfree_rcu so that nft core can
* can check expr->ops->size even after nft_compat->destroy() frees * can check expr->ops->size even after nft_compat->destroy() frees
...@@ -61,7 +64,7 @@ static struct nft_compat_net *nft_compat_pernet(struct net *net) ...@@ -61,7 +64,7 @@ static struct nft_compat_net *nft_compat_pernet(struct net *net)
static bool nft_xt_put(struct nft_xt *xt) static bool nft_xt_put(struct nft_xt *xt)
{ {
if (refcount_dec_and_test(&xt->refcnt)) { if (refcount_dec_and_test(&xt->refcnt)) {
list_del(&xt->head); WARN_ON_ONCE(!list_empty(&xt->head));
kfree_rcu(xt, rcu_head); kfree_rcu(xt, rcu_head);
return true; return true;
} }
...@@ -555,6 +558,43 @@ nft_match_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) ...@@ -555,6 +558,43 @@ nft_match_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr)
__nft_match_destroy(ctx, expr, nft_expr_priv(expr)); __nft_match_destroy(ctx, expr, nft_expr_priv(expr));
} }
static void nft_compat_activate(const struct nft_ctx *ctx,
const struct nft_expr *expr,
struct list_head *h)
{
struct nft_xt *xt = container_of(expr->ops, struct nft_xt, ops);
if (xt->listcnt == 0)
list_add(&xt->head, h);
xt->listcnt++;
}
static void nft_compat_activate_mt(const struct nft_ctx *ctx,
const struct nft_expr *expr)
{
struct nft_compat_net *cn = nft_compat_pernet(ctx->net);
nft_compat_activate(ctx, expr, &cn->nft_match_list);
}
static void nft_compat_activate_tg(const struct nft_ctx *ctx,
const struct nft_expr *expr)
{
struct nft_compat_net *cn = nft_compat_pernet(ctx->net);
nft_compat_activate(ctx, expr, &cn->nft_target_list);
}
static void nft_compat_deactivate(const struct nft_ctx *ctx,
const struct nft_expr *expr)
{
struct nft_xt *xt = container_of(expr->ops, struct nft_xt, ops);
if (--xt->listcnt == 0)
list_del_init(&xt->head);
}
static void static void
nft_match_large_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr) nft_match_large_destroy(const struct nft_ctx *ctx, const struct nft_expr *expr)
{ {
...@@ -808,6 +848,8 @@ nft_match_select_ops(const struct nft_ctx *ctx, ...@@ -808,6 +848,8 @@ nft_match_select_ops(const struct nft_ctx *ctx,
nft_match->ops.eval = nft_match_eval; nft_match->ops.eval = nft_match_eval;
nft_match->ops.init = nft_match_init; nft_match->ops.init = nft_match_init;
nft_match->ops.destroy = nft_match_destroy; nft_match->ops.destroy = nft_match_destroy;
nft_match->ops.activate = nft_compat_activate_mt;
nft_match->ops.deactivate = nft_compat_deactivate;
nft_match->ops.dump = nft_match_dump; nft_match->ops.dump = nft_match_dump;
nft_match->ops.validate = nft_match_validate; nft_match->ops.validate = nft_match_validate;
nft_match->ops.data = match; nft_match->ops.data = match;
...@@ -824,6 +866,7 @@ nft_match_select_ops(const struct nft_ctx *ctx, ...@@ -824,6 +866,7 @@ nft_match_select_ops(const struct nft_ctx *ctx,
nft_match->ops.size = matchsize; nft_match->ops.size = matchsize;
nft_match->listcnt = 1;
list_add(&nft_match->head, &cn->nft_match_list); list_add(&nft_match->head, &cn->nft_match_list);
return &nft_match->ops; return &nft_match->ops;
...@@ -910,6 +953,8 @@ nft_target_select_ops(const struct nft_ctx *ctx, ...@@ -910,6 +953,8 @@ nft_target_select_ops(const struct nft_ctx *ctx,
nft_target->ops.size = NFT_EXPR_SIZE(XT_ALIGN(target->targetsize)); nft_target->ops.size = NFT_EXPR_SIZE(XT_ALIGN(target->targetsize));
nft_target->ops.init = nft_target_init; nft_target->ops.init = nft_target_init;
nft_target->ops.destroy = nft_target_destroy; nft_target->ops.destroy = nft_target_destroy;
nft_target->ops.activate = nft_compat_activate_tg;
nft_target->ops.deactivate = nft_compat_deactivate;
nft_target->ops.dump = nft_target_dump; nft_target->ops.dump = nft_target_dump;
nft_target->ops.validate = nft_target_validate; nft_target->ops.validate = nft_target_validate;
nft_target->ops.data = target; nft_target->ops.data = target;
...@@ -919,6 +964,7 @@ nft_target_select_ops(const struct nft_ctx *ctx, ...@@ -919,6 +964,7 @@ nft_target_select_ops(const struct nft_ctx *ctx,
else else
nft_target->ops.eval = nft_target_eval_xt; nft_target->ops.eval = nft_target_eval_xt;
nft_target->listcnt = 1;
list_add(&nft_target->head, &cn->nft_target_list); list_add(&nft_target->head, &cn->nft_target_list);
return &nft_target->ops; return &nft_target->ops;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment