[PATCH nft] evaluate: don't update cache for anonymous chains

[PATCH nft] evaluate: don't update cache for anonymous chains

[Florian Westphal]

Chain lookup needs a name, not a numerical id.
After patch, loading bogon gives following errors:

Error: No such file or directory chain c {
Error: No symbol type information a b index 1 10.1.26.a

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/evaluate.c                                            | 3 +++
 .../bogons/nft-f/null_deref_on_anon_chain_update_crash    | 8 ++++++++
 2 files changed, 11 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..09df7f158acc 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -5371,6 +5371,9 @@ static int rule_cache_update(struct eval_ctx *ctx, enum cmd_ops op)
 	if (!table)
 		return table_not_found(ctx);
 
+	if (!rule->handle.chain.name)
+		return chain_not_found(ctx);
+
 	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

Re: [PATCH nft] evaluate: don't update cache for anonymous chains

[Pablo Neira Ayuso]

On Fri, Mar 21, 2025 at 12:46:38PM +0100, Florian Westphal wrote:
> Chain lookup needs a name, not a numerical id.
> After patch, loading bogon gives following errors:
> 
> Error: No such file or directory chain c {
> Error: No symbol type information a b index 1 10.1.26.a
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  src/evaluate.c                                            | 3 +++
>  .../bogons/nft-f/null_deref_on_anon_chain_update_crash    | 8 ++++++++
>  2 files changed, 11 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..09df7f158acc 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -5371,6 +5371,9 @@ static int rule_cache_update(struct eval_ctx *ctx, enum cmd_ops op)
>  	if (!table)
>  		return table_not_found(ctx);
>  
> +	if (!rule->handle.chain.name)
> +		return chain_not_found(ctx);

rule_cache_update() is invoked because of index, which toggle
NFT_CACHE_UPDATE.

Maybe rule_cache_update() should be skipped for anonymous chain
instead? ie. 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
> 
> 

Re: [PATCH nft] evaluate: don't update cache for anonymous chains

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> rule_cache_update() is invoked because of index, which toggle
> NFT_CACHE_UPDATE.
> 
> Maybe rule_cache_update() should be skipped for anonymous chain
> instead? ie. return 0.

Makes sense, i sent a v2 and added a comment wrt. anon chains
being immutable.