| Message ID | 20250321143008.7980-1-fw@strlen.de |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [nft,v2] evaluate: don't update cache for anonymous chains | expand |
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@strlen.de> > --- > 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 > >
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. + */ + 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
Chain lookup needs a name, not a numerical id. 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) Signed-off-by: Florian Westphal <fw@strlen.de> --- 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