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

netfilter: nf_tables: delay chain policy update until transaction is complete

When we process a long ruleset of the form

chain input {
   type filter hook input priority filter; policy drop;
   ...
}

Then the base chain gets registered early on, we then continue to
process/validate the next messages coming in the same transaction.

Problem is that if the base chain policy is 'drop', it will take effect
immediately, which causes all traffic to get blocked until the
transaction completes or is aborted.

Fix this by deferring the policy until the transaction has been
processed and all of the rules have been flagged as active.
Reported-by: default avatarJann Haber <jann.haber@selfnet.de>
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent 6c0afef5
...@@ -214,33 +214,33 @@ static int nft_deltable(struct nft_ctx *ctx) ...@@ -214,33 +214,33 @@ static int nft_deltable(struct nft_ctx *ctx)
return err; return err;
} }
static int nft_trans_chain_add(struct nft_ctx *ctx, int msg_type) static struct nft_trans *nft_trans_chain_add(struct nft_ctx *ctx, int msg_type)
{ {
struct nft_trans *trans; struct nft_trans *trans;
trans = nft_trans_alloc(ctx, msg_type, sizeof(struct nft_trans_chain)); trans = nft_trans_alloc(ctx, msg_type, sizeof(struct nft_trans_chain));
if (trans == NULL) if (trans == NULL)
return -ENOMEM; return ERR_PTR(-ENOMEM);
if (msg_type == NFT_MSG_NEWCHAIN) if (msg_type == NFT_MSG_NEWCHAIN)
nft_activate_next(ctx->net, ctx->chain); nft_activate_next(ctx->net, ctx->chain);
list_add_tail(&trans->list, &ctx->net->nft.commit_list); list_add_tail(&trans->list, &ctx->net->nft.commit_list);
return 0; return trans;
} }
static int nft_delchain(struct nft_ctx *ctx) static int nft_delchain(struct nft_ctx *ctx)
{ {
int err; struct nft_trans *trans;
err = nft_trans_chain_add(ctx, NFT_MSG_DELCHAIN); trans = nft_trans_chain_add(ctx, NFT_MSG_DELCHAIN);
if (err < 0) if (IS_ERR(trans))
return err; return PTR_ERR(trans);
ctx->table->use--; ctx->table->use--;
nft_deactivate_next(ctx->net, ctx->chain); nft_deactivate_next(ctx->net, ctx->chain);
return err; return 0;
} }
static void nft_rule_expr_activate(const struct nft_ctx *ctx, static void nft_rule_expr_activate(const struct nft_ctx *ctx,
...@@ -1615,6 +1615,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask, ...@@ -1615,6 +1615,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
struct nft_base_chain *basechain; struct nft_base_chain *basechain;
struct nft_stats __percpu *stats; struct nft_stats __percpu *stats;
struct net *net = ctx->net; struct net *net = ctx->net;
struct nft_trans *trans;
struct nft_chain *chain; struct nft_chain *chain;
struct nft_rule **rules; struct nft_rule **rules;
int err; int err;
...@@ -1662,7 +1663,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask, ...@@ -1662,7 +1663,7 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
ops->dev = hook.dev; ops->dev = hook.dev;
chain->flags |= NFT_BASE_CHAIN; chain->flags |= NFT_BASE_CHAIN;
basechain->policy = policy; basechain->policy = NF_ACCEPT;
} else { } else {
chain = kzalloc(sizeof(*chain), GFP_KERNEL); chain = kzalloc(sizeof(*chain), GFP_KERNEL);
if (chain == NULL) if (chain == NULL)
...@@ -1698,13 +1699,18 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask, ...@@ -1698,13 +1699,18 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask,
if (err) if (err)
goto err2; goto err2;
err = nft_trans_chain_add(ctx, NFT_MSG_NEWCHAIN); trans = nft_trans_chain_add(ctx, NFT_MSG_NEWCHAIN);
if (err < 0) { if (IS_ERR(trans)) {
err = PTR_ERR(trans);
rhltable_remove(&table->chains_ht, &chain->rhlhead, rhltable_remove(&table->chains_ht, &chain->rhlhead,
nft_chain_ht_params); nft_chain_ht_params);
goto err2; goto err2;
} }
nft_trans_chain_policy(trans) = -1;
if (nft_is_base_chain(chain))
nft_trans_chain_policy(trans) = policy;
table->use++; table->use++;
list_add_tail_rcu(&chain->list, &table->chains); list_add_tail_rcu(&chain->list, &table->chains);
...@@ -6311,6 +6317,27 @@ static int nf_tables_validate(struct net *net) ...@@ -6311,6 +6317,27 @@ static int nf_tables_validate(struct net *net)
return 0; return 0;
} }
/* a drop policy has to be deferred until all rules have been activated,
* otherwise a large ruleset that contains a drop-policy base chain will
* cause all packets to get dropped until the full transaction has been
* processed.
*
* We defer the drop policy until the transaction has been finalized.
*/
static void nft_chain_commit_drop_policy(struct nft_trans *trans)
{
struct nft_base_chain *basechain;
if (nft_trans_chain_policy(trans) != NF_DROP)
return;
if (!nft_is_base_chain(trans->ctx.chain))
return;
basechain = nft_base_chain(trans->ctx.chain);
basechain->policy = NF_DROP;
}
static void nft_chain_commit_update(struct nft_trans *trans) static void nft_chain_commit_update(struct nft_trans *trans)
{ {
struct nft_base_chain *basechain; struct nft_base_chain *basechain;
...@@ -6632,6 +6659,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb) ...@@ -6632,6 +6659,7 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN); nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN);
/* trans destroyed after rcu grace period */ /* trans destroyed after rcu grace period */
} else { } else {
nft_chain_commit_drop_policy(trans);
nft_clear(net, trans->ctx.chain); nft_clear(net, trans->ctx.chain);
nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN); nf_tables_chain_notify(&trans->ctx, NFT_MSG_NEWCHAIN);
nft_trans_destroy(trans); nft_trans_destroy(trans);
......
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