On Fri, Mar 21, 2025 at 03:30:05PM +0100, Florian Westphal wrote:
> Chain lookup needs a name, not a numerical id.
Thanks for this v2.
> After patch, loading bogon gives following errors:
>
> Error: No symbol type information a b index 1 10.1.26.a
>
> v2: Don't return an error, just make it a no-op (Pablo Neira Ayuso)
Fixes: c330152b7f77 ("src: support for implicit chain bindings")
I suggest to modify the comment below before you apply:
> Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
> ---
> src/evaluate.c | 7 +++++++
> .../bogons/nft-f/null_deref_on_anon_chain_update_crash | 8 ++++++++
> 2 files changed, 15 insertions(+)
> create mode 100644 tests/shell/testcases/bogons/nft-f/null_deref_on_anon_chain_update_crash
>
> diff --git a/src/evaluate.c b/src/evaluate.c
> index a27961193da5..26aa0ef53241 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -5371,6 +5371,13 @@ static int rule_cache_update(struct eval_ctx *ctx, enum cmd_ops op)
> if (!table)
> return table_not_found(ctx);
>
> + /* chain is anonymous: no update needed, rules cannot be added
> + * or removed from anon chains after the chain was committed to
> + * kernel.
> + */
I suggest this comment:
/* chain is anonymous, adding new rules via index is not supported. */
note that this bogon test you provide is actually putting everything
in the same transaction. Your comment regarding kernel behaviour is
correct, but it does not apply to rule_cache_update(). The issue is
that rule_cache_update() does not need to update the cache if this
chain is anonymous.
Thanks.
> + if (!rule->handle.chain.name)
> + return 0;
> +
> chain = chain_cache_find(table, rule->handle.chain.name);
> if (!chain)
> return chain_not_found(ctx);
> diff --git a/tests/shell/testcases/bogons/nft-f/null_deref_on_anon_chain_update_crash b/tests/shell/testcases/bogons/nft-f/null_deref_on_anon_chain_update_crash
> new file mode 100644
> index 000000000000..310486c59ee0
> --- /dev/null
> +++ b/tests/shell/testcases/bogons/nft-f/null_deref_on_anon_chain_update_crash
> @@ -0,0 +1,8 @@
> +table ip f {
> + chain c {
> + jump {
> + accept
> + }
> + }
> +}
> +a b index 1 10.1.26.a
> --
> 2.48.1
>
>
