Skip to content

Commit

Permalink
Merge tag 'nf-24-07-11' of git://git.kernel.org/pub/scm/linux/kernel/…
Browse files Browse the repository at this point in the history
…git/netfilter/nf

Pablo Neira Ayuso says:

====================
Netfilter fixes for net

The following batch contains Netfilter fixes for net:

Patch #1 fixes a bogus WARN_ON splat in nfnetlink_queue.

Patch #2 fixes a crash due to stack overflow in chain loop detection
	 by using the existing chain validation routines

Both patches from Florian Westphal.

netfilter pull request 24-07-11

* tag 'nf-24-07-11' of git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf:
  netfilter: nf_tables: prefer nft_chain_validate
  netfilter: nfnetlink_queue: drop bogus WARN_ON
====================

Link: https://patch.msgid.link/20240711093948.3816-1-pablo@netfilter.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
  • Loading branch information
Paolo Abeni committed Jul 11, 2024
2 parents a819ff0 + cff3bd0 commit d7c199e
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 146 deletions.
158 changes: 13 additions & 145 deletions net/netfilter/nf_tables_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -3823,6 +3823,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;
Expand All @@ -3844,6 +3853,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;
Expand Down Expand Up @@ -10809,150 +10821,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_elem_priv *elem_priv)
{
const struct nft_set_ext *ext = nft_set_elem_ext(set, elem_priv);

if (!nft_set_elem_active(ext, iter->genmask))
return 0;

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;

if (fatal_signal_pending(current))
return -EINTR;

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.type = NFT_ITER_UPDATE;
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
*
Expand Down Expand Up @@ -11065,7 +10933,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;
}
Expand Down
2 changes: 1 addition & 1 deletion net/netfilter/nfnetlink_queue.c
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ static void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict)
hooks = nf_hook_entries_head(net, pf, entry->state.hook);

i = entry->hook_index;
if (WARN_ON_ONCE(!hooks || i >= hooks->num_hook_entries)) {
if (!hooks || i >= hooks->num_hook_entries) {
kfree_skb_reason(skb, SKB_DROP_REASON_NETFILTER_DROP);
nf_queue_entry_free(entry);
return;
Expand Down

0 comments on commit d7c199e

Please sign in to comment.