netfilter: nf_tables: prefer nft_chain_validate
commit cff3bd012a9512ac5ed858d38e6ed65f6391008c upstream.
nft_chain_validate already performs loop detection because a cycle will
result in a call stack overflow (ctx->level >= NFT_JUMP_STACK_SIZE).
It also follows maps via ->validate callback in nft_lookup, so there
appears no reason to iterate the maps again.
nf_tables_check_loops() and all its helper functions can be removed.
This improves ruleset load time significantly, from 23s down to 12s.
This also fixes a crash bug. Old loop detection code can result in
unbounded recursion:
BUG: TASK stack guard page was hit at ....
Oops: stack guard page: 0000 [#1] PREEMPT SMP KASAN
CPU: 4 PID: 1539 Comm: nft Not tainted 6.10.0-rc5+ #1
[..]
with a suitable ruleset during validation of register stores.
I can't see any actual reason to attempt to check for this from
nft_validate_register_store(), at this point the transaction is still in
progress, so we don't have a full picture of the rule graph.
For nf-next it might make sense to either remove it or make this depend
on table->validate_state in case we could catch an error earlier
(for improved error reporting to userspace).
Fixes: 20a69341f2
("netfilter: nf_tables: add netlink set API")
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Родитель
d7c5f8bd12
Коммит
8246b7466c
|
@ -3445,6 +3445,15 @@ static void nf_tables_rule_release(const struct nft_ctx *ctx, struct nft_rule *r
|
|||
nf_tables_rule_destroy(ctx, rule);
|
||||
}
|
||||
|
||||
/** nft_chain_validate - loop detection and hook validation
|
||||
*
|
||||
* @ctx: context containing call depth and base chain
|
||||
* @chain: chain to validate
|
||||
*
|
||||
* Walk through the rules of the given chain and chase all jumps/gotos
|
||||
* and set lookups until either the jump limit is hit or all reachable
|
||||
* chains have been validated.
|
||||
*/
|
||||
int nft_chain_validate(const struct nft_ctx *ctx, const struct nft_chain *chain)
|
||||
{
|
||||
struct nft_expr *expr, *last;
|
||||
|
@ -3463,6 +3472,9 @@ int nft_chain_validate(const struct nft_ctx *ctx, const struct nft_chain *chain)
|
|||
if (!expr->ops->validate)
|
||||
continue;
|
||||
|
||||
/* This may call nft_chain_validate() recursively,
|
||||
* callers that do so must increment ctx->level.
|
||||
*/
|
||||
err = expr->ops->validate(ctx, expr, &data);
|
||||
if (err < 0)
|
||||
return err;
|
||||
|
@ -9999,143 +10011,6 @@ int nft_chain_validate_hooks(const struct nft_chain *chain,
|
|||
}
|
||||
EXPORT_SYMBOL_GPL(nft_chain_validate_hooks);
|
||||
|
||||
/*
|
||||
* Loop detection - walk through the ruleset beginning at the destination chain
|
||||
* of a new jump until either the source chain is reached (loop) or all
|
||||
* reachable chains have been traversed.
|
||||
*
|
||||
* The loop check is performed whenever a new jump verdict is added to an
|
||||
* expression or verdict map or a verdict map is bound to a new chain.
|
||||
*/
|
||||
|
||||
static int nf_tables_check_loops(const struct nft_ctx *ctx,
|
||||
const struct nft_chain *chain);
|
||||
|
||||
static int nft_check_loops(const struct nft_ctx *ctx,
|
||||
const struct nft_set_ext *ext)
|
||||
{
|
||||
const struct nft_data *data;
|
||||
int ret;
|
||||
|
||||
data = nft_set_ext_data(ext);
|
||||
switch (data->verdict.code) {
|
||||
case NFT_JUMP:
|
||||
case NFT_GOTO:
|
||||
ret = nf_tables_check_loops(ctx, data->verdict.chain);
|
||||
break;
|
||||
default:
|
||||
ret = 0;
|
||||
break;
|
||||
}
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
static int nf_tables_loop_check_setelem(const struct nft_ctx *ctx,
|
||||
struct nft_set *set,
|
||||
const struct nft_set_iter *iter,
|
||||
struct nft_set_elem *elem)
|
||||
{
|
||||
const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv);
|
||||
|
||||
if (nft_set_ext_exists(ext, NFT_SET_EXT_FLAGS) &&
|
||||
*nft_set_ext_flags(ext) & NFT_SET_ELEM_INTERVAL_END)
|
||||
return 0;
|
||||
|
||||
return nft_check_loops(ctx, ext);
|
||||
}
|
||||
|
||||
static int nft_set_catchall_loops(const struct nft_ctx *ctx,
|
||||
struct nft_set *set)
|
||||
{
|
||||
u8 genmask = nft_genmask_next(ctx->net);
|
||||
struct nft_set_elem_catchall *catchall;
|
||||
struct nft_set_ext *ext;
|
||||
int ret = 0;
|
||||
|
||||
list_for_each_entry_rcu(catchall, &set->catchall_list, list) {
|
||||
ext = nft_set_elem_ext(set, catchall->elem);
|
||||
if (!nft_set_elem_active(ext, genmask))
|
||||
continue;
|
||||
|
||||
ret = nft_check_loops(ctx, ext);
|
||||
if (ret < 0)
|
||||
return ret;
|
||||
}
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
static int nf_tables_check_loops(const struct nft_ctx *ctx,
|
||||
const struct nft_chain *chain)
|
||||
{
|
||||
const struct nft_rule *rule;
|
||||
const struct nft_expr *expr, *last;
|
||||
struct nft_set *set;
|
||||
struct nft_set_binding *binding;
|
||||
struct nft_set_iter iter;
|
||||
|
||||
if (ctx->chain == chain)
|
||||
return -ELOOP;
|
||||
|
||||
list_for_each_entry(rule, &chain->rules, list) {
|
||||
nft_rule_for_each_expr(expr, last, rule) {
|
||||
struct nft_immediate_expr *priv;
|
||||
const struct nft_data *data;
|
||||
int err;
|
||||
|
||||
if (strcmp(expr->ops->type->name, "immediate"))
|
||||
continue;
|
||||
|
||||
priv = nft_expr_priv(expr);
|
||||
if (priv->dreg != NFT_REG_VERDICT)
|
||||
continue;
|
||||
|
||||
data = &priv->data;
|
||||
switch (data->verdict.code) {
|
||||
case NFT_JUMP:
|
||||
case NFT_GOTO:
|
||||
err = nf_tables_check_loops(ctx,
|
||||
data->verdict.chain);
|
||||
if (err < 0)
|
||||
return err;
|
||||
break;
|
||||
default:
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
list_for_each_entry(set, &ctx->table->sets, list) {
|
||||
if (!nft_is_active_next(ctx->net, set))
|
||||
continue;
|
||||
if (!(set->flags & NFT_SET_MAP) ||
|
||||
set->dtype != NFT_DATA_VERDICT)
|
||||
continue;
|
||||
|
||||
list_for_each_entry(binding, &set->bindings, list) {
|
||||
if (!(binding->flags & NFT_SET_MAP) ||
|
||||
binding->chain != chain)
|
||||
continue;
|
||||
|
||||
iter.genmask = nft_genmask_next(ctx->net);
|
||||
iter.skip = 0;
|
||||
iter.count = 0;
|
||||
iter.err = 0;
|
||||
iter.fn = nf_tables_loop_check_setelem;
|
||||
|
||||
set->ops->walk(ctx, set, &iter);
|
||||
if (!iter.err)
|
||||
iter.err = nft_set_catchall_loops(ctx, set);
|
||||
|
||||
if (iter.err < 0)
|
||||
return iter.err;
|
||||
}
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* nft_parse_u32_check - fetch u32 attribute and check for maximum value
|
||||
*
|
||||
|
@ -10248,7 +10123,7 @@ static int nft_validate_register_store(const struct nft_ctx *ctx,
|
|||
if (data != NULL &&
|
||||
(data->verdict.code == NFT_GOTO ||
|
||||
data->verdict.code == NFT_JUMP)) {
|
||||
err = nf_tables_check_loops(ctx, data->verdict.chain);
|
||||
err = nft_chain_validate(ctx, data->verdict.chain);
|
||||
if (err < 0)
|
||||
return err;
|
||||
}
|
||||
|
|
Загрузка…
Ссылка в новой задаче