Hi

Dropping some recipients for the Debian specific handling of this
issue. So AFAIU upstream will not consider this on src:linux side to
be further handled and needs to be addressed in nftables.

Arturo: With the patches provided I prepared (as Timo) an update
targetting bookworm for the next point release (bug for release.d.o to
be submitted soon).

Attached is the proposed debdiff, ans as well as MR on salsa.

https://salsa.debian.org/pkg-netfilter-team/pkg-nftables/-/merge_requests/11
(note not touching thte salsa-ci part was deliberate, but to make the
piuparts test one would need adjustment of the target release. But as
itwas not done for the +deb12u1 itself, I have not touched this)

The same would be needed OTOH for bullseye as well.

Regards,
Salvatore
diff -Nru nftables-1.0.6/debian/changelog nftables-1.0.6/debian/changelog
--- nftables-1.0.6/debian/changelog     2023-06-20 16:55:52.000000000 +0200
+++ nftables-1.0.6/debian/changelog     2023-09-16 07:47:15.000000000 +0200
@@ -1,3 +1,13 @@
+nftables (1.0.6-2+deb12u2) bookworm; urgency=medium
+
+  * [136245a] Fix incorrect bytecode generation hit with new kernel check that
+    rejects adding rules to bound chains (Closes: #1051592)
+    - rule: add helper function to expand chain rules intoi commands
+    - rule: expand standalone chain that contains rules
+    - src: expand table command before evaluation
+
+ -- Salvatore Bonaccorso <car...@debian.org>  Sat, 16 Sep 2023 07:47:15 +0200
+
 nftables (1.0.6-2+deb12u1) bookworm; urgency=medium
 
   * [7edf72e] d/patches: add 0001-debian-bug-1038724.patch (Closes: #1038724)
diff -Nru 
nftables-1.0.6/debian/patches/rule-add-helper-function-to-expand-chain-rules-into-.patch
 
nftables-1.0.6/debian/patches/rule-add-helper-function-to-expand-chain-rules-into-.patch
--- 
nftables-1.0.6/debian/patches/rule-add-helper-function-to-expand-chain-rules-into-.patch
    1970-01-01 01:00:00.000000000 +0100
+++ 
nftables-1.0.6/debian/patches/rule-add-helper-function-to-expand-chain-rules-into-.patch
    2023-09-16 07:47:15.000000000 +0200
@@ -0,0 +1,82 @@
+From 4e5b0a64227dde250f94bec45b3fb127d78b7fd2 Mon Sep 17 00:00:00 2001
+From: Pablo Neira Ayuso <pa...@netfilter.org>
+Date: Mon, 6 Feb 2023 15:28:40 +0100
+Subject: [PATCH 1/3,nft] rule: add helper function to expand chain rules intoi
+ commands
+
+[ upstream commit 784597a4ed63b9decb10d74fdb49a1b021e22728 ]
+
+This patch adds a helper function to expand chain rules into commands.
+This comes in preparation for the follow up patch.
+
+Signed-off-by: Pablo Neira Ayuso <pa...@netfilter.org>
+---
+ src/rule.c | 39 ++++++++++++++++++++++-----------------
+ 1 file changed, 22 insertions(+), 17 deletions(-)
+
+diff --git a/src/rule.c b/src/rule.c
+index 1402210acd8d..43c6520517ce 100644
+--- a/src/rule.c
++++ b/src/rule.c
+@@ -1310,13 +1310,31 @@ void cmd_add_loc(struct cmd *cmd, uint16_t offset, 
const struct location *loc)
+       cmd->num_attrs++;
+ }
+ 
++static void nft_cmd_expand_chain(struct chain *chain, struct list_head 
*new_cmds)
++{
++      struct rule *rule;
++      struct handle h;
++      struct cmd *new;
++
++      list_for_each_entry(rule, &chain->rules, list) {
++              memset(&h, 0, sizeof(h));
++              handle_merge(&h, &rule->handle);
++              if (chain->flags & CHAIN_F_BINDING) {
++                      rule->handle.chain_id = chain->handle.chain_id;
++                      rule->handle.chain.location = chain->location;
++              }
++              new = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, &h,
++                              &rule->location, rule_get(rule));
++              list_add_tail(&new->list, new_cmds);
++      }
++}
++
+ void nft_cmd_expand(struct cmd *cmd)
+ {
+       struct list_head new_cmds;
+       struct flowtable *ft;
+       struct table *table;
+       struct chain *chain;
+-      struct rule *rule;
+       struct set *set;
+       struct obj *obj;
+       struct cmd *new;
+@@ -1362,22 +1380,9 @@ void nft_cmd_expand(struct cmd *cmd)
+                                       &ft->location, flowtable_get(ft));
+                       list_add_tail(&new->list, &new_cmds);
+               }
+-              list_for_each_entry(chain, &table->chains, list) {
+-                      list_for_each_entry(rule, &chain->rules, list) {
+-                              memset(&h, 0, sizeof(h));
+-                              handle_merge(&h, &rule->handle);
+-                              if (chain->flags & CHAIN_F_BINDING) {
+-                                      rule->handle.chain_id =
+-                                              chain->handle.chain_id;
+-                                      rule->handle.chain.location =
+-                                              chain->location;
+-                              }
+-                              new = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, &h,
+-                                              &rule->location,
+-                                              rule_get(rule));
+-                              list_add_tail(&new->list, &new_cmds);
+-                      }
+-              }
++              list_for_each_entry(chain, &table->chains, list)
++                      nft_cmd_expand_chain(chain, &new_cmds);
++
+               list_splice(&new_cmds, &cmd->list);
+               break;
+       case CMD_OBJ_SET:
+-- 
+2.30.2
+
diff -Nru 
nftables-1.0.6/debian/patches/rule-expand-standalone-chain-that-contains-rules.patch
 
nftables-1.0.6/debian/patches/rule-expand-standalone-chain-that-contains-rules.patch
--- 
nftables-1.0.6/debian/patches/rule-expand-standalone-chain-that-contains-rules.patch
        1970-01-01 01:00:00.000000000 +0100
+++ 
nftables-1.0.6/debian/patches/rule-expand-standalone-chain-that-contains-rules.patch
        2023-09-16 07:47:15.000000000 +0200
@@ -0,0 +1,115 @@
+From 70c03d81df0e87fb416bd1f38409367e9d08ed7f Mon Sep 17 00:00:00 2001
+From: Pablo Neira Ayuso <pa...@netfilter.org>
+Date: Mon, 6 Feb 2023 15:28:41 +0100
+Subject: [PATCH 2/3,nft] rule: expand standalone chain that contains rules
+
+[ upstream 27c753e4a8d4744f479345e3f5e34cafef751602 commit ]
+
+Otherwise rules that this chain contains are ignored when expressed
+using the following syntax:
+
+chain inet filter input2 {
+       type filter hook input priority filter; policy accept;
+       ip saddr 1.2.3.4 tcp dport { 22, 443, 123 } drop
+}
+
+When expanding the chain, remove the rule so the new CMD_OBJ_CHAIN
+case does not expand it again.
+
+Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1655
+Signed-off-by: Pablo Neira Ayuso <pa...@netfilter.org>
+---
+ src/rule.c                                    | 15 +++++++++---
+ .../testcases/include/0020include_chain_0     | 23 +++++++++++++++++++
+ .../include/dumps/0020include_chain_0.nft     |  6 +++++
+ 3 files changed, 41 insertions(+), 3 deletions(-)
+ create mode 100755 tests/shell/testcases/include/0020include_chain_0
+ create mode 100644 tests/shell/testcases/include/dumps/0020include_chain_0.nft
+
+diff --git a/src/rule.c b/src/rule.c
+index 43c6520517ce..323d89afd141 100644
+--- a/src/rule.c
++++ b/src/rule.c
+@@ -1312,11 +1312,12 @@ void cmd_add_loc(struct cmd *cmd, uint16_t offset, 
const struct location *loc)
+ 
+ static void nft_cmd_expand_chain(struct chain *chain, struct list_head 
*new_cmds)
+ {
+-      struct rule *rule;
++      struct rule *rule, *next;
+       struct handle h;
+       struct cmd *new;
+ 
+-      list_for_each_entry(rule, &chain->rules, list) {
++      list_for_each_entry_safe(rule, next, &chain->rules, list) {
++              list_del(&rule->list);
+               memset(&h, 0, sizeof(h));
+               handle_merge(&h, &rule->handle);
+               if (chain->flags & CHAIN_F_BINDING) {
+@@ -1324,7 +1325,7 @@ static void nft_cmd_expand_chain(struct chain *chain, 
struct list_head *new_cmds
+                       rule->handle.chain.location = chain->location;
+               }
+               new = cmd_alloc(CMD_ADD, CMD_OBJ_RULE, &h,
+-                              &rule->location, rule_get(rule));
++                              &rule->location, rule);
+               list_add_tail(&new->list, new_cmds);
+       }
+ }
+@@ -1385,6 +1386,14 @@ void nft_cmd_expand(struct cmd *cmd)
+ 
+               list_splice(&new_cmds, &cmd->list);
+               break;
++      case CMD_OBJ_CHAIN:
++              chain = cmd->chain;
++              if (!chain || list_empty(&chain->rules))
++                      break;
++
++              nft_cmd_expand_chain(chain, &new_cmds);
++              list_splice(&new_cmds, &cmd->list);
++              break;
+       case CMD_OBJ_SET:
+       case CMD_OBJ_MAP:
+               set = cmd->set;
+diff --git a/tests/shell/testcases/include/0020include_chain_0 
b/tests/shell/testcases/include/0020include_chain_0
+new file mode 100755
+index 000000000000..8f78e8c606ec
+--- /dev/null
++++ b/tests/shell/testcases/include/0020include_chain_0
+@@ -0,0 +1,23 @@
++#!/bin/bash
++
++set -e
++
++tmpfile1=$(mktemp -p .)
++if [ ! -w $tmpfile1 ] ; then
++      echo "Failed to create tmp file" >&2
++      exit 0
++fi
++
++trap "rm -rf $tmpfile1" EXIT # cleanup if aborted
++
++RULESET="table inet filter { }
++include \"$tmpfile1\""
++
++RULESET2="chain inet filter input2 {
++      type filter hook input priority filter; policy accept;
++      ip saddr 1.2.3.4 tcp dport { 22, 443, 123 } drop
++}"
++
++echo "$RULESET2" > $tmpfile1
++
++$NFT -o -f - <<< $RULESET
+diff --git a/tests/shell/testcases/include/dumps/0020include_chain_0.nft 
b/tests/shell/testcases/include/dumps/0020include_chain_0.nft
+new file mode 100644
+index 000000000000..3ad6db14d2f5
+--- /dev/null
++++ b/tests/shell/testcases/include/dumps/0020include_chain_0.nft
+@@ -0,0 +1,6 @@
++table inet filter {
++      chain input2 {
++              type filter hook input priority filter; policy accept;
++              ip saddr 1.2.3.4 tcp dport { 22, 123, 443 } drop
++      }
++}
+-- 
+2.30.2
+
diff -Nru nftables-1.0.6/debian/patches/series 
nftables-1.0.6/debian/patches/series
--- nftables-1.0.6/debian/patches/series        2023-06-20 16:55:52.000000000 
+0200
+++ nftables-1.0.6/debian/patches/series        2023-09-16 07:47:15.000000000 
+0200
@@ -1,2 +1,5 @@
 0001-debian-bug-1038724.patch
 invalid-octal-fix.patch
+rule-add-helper-function-to-expand-chain-rules-into-.patch
+rule-expand-standalone-chain-that-contains-rules.patch
+src-expand-table-command-before-evaluation.patch
diff -Nru 
nftables-1.0.6/debian/patches/src-expand-table-command-before-evaluation.patch 
nftables-1.0.6/debian/patches/src-expand-table-command-before-evaluation.patch
--- 
nftables-1.0.6/debian/patches/src-expand-table-command-before-evaluation.patch  
    1970-01-01 01:00:00.000000000 +0100
+++ 
nftables-1.0.6/debian/patches/src-expand-table-command-before-evaluation.patch  
    2023-09-16 07:47:15.000000000 +0200
@@ -0,0 +1,201 @@
+From a13ffae3838548d9b337d6efe74af070edc355d5 Mon Sep 17 00:00:00 2001
+From: Pablo Neira Ayuso <pa...@netfilter.org>
+Date: Thu, 23 Feb 2023 19:55:39 +0100
+Subject: [PATCH 3/3,nft] src: expand table command before evaluation
+
+[ upstream 3975430b12d97c92cdf03753342f2269153d5624 commit ]
+
+The nested syntax notation results in one single table command which
+includes all other objects. This differs from the flat notation where
+there is usually one command per object.
+
+This patch adds a previous step to the evaluation phase to expand the
+objects that are contained in the table into independent commands, so
+both notations have similar representations.
+
+Remove the code to evaluate the nested representation in the evaluation
+phase since commands are independently evaluated after the expansion.
+
+The commands are expanded after the set element collapse step, in case
+that there is a long list of singleton element commands to be added to
+the set, to shorten the command list iteration.
+
+This approach also avoids interference with the object cache that is
+populated in the evaluation, which might refer to objects coming in the
+existing command list that is being processed.
+
+There is still a post_expand phase to detach the elements from the set
+which could be consolidated by updating the evaluation step to handle
+the CMD_OBJ_SETELEMS command type.
+
+This patch fixes 27c753e4a8d4 ("rule: expand standalone chain that
+contains rules") which broke rule addition/insertion by index because
+the expansion code after the evaluation messes up the cache.
+
+Fixes: 27c753e4a8d4 ("rule: expand standalone chain that contains rules")
+Signed-off-by: Pablo Neira Ayuso <pa...@netfilter.org>
+---
+ include/rule.h    |  1 +
+ src/evaluate.c    | 39 ---------------------------------------
+ src/libnftables.c |  9 ++++++++-
+ src/rule.c        | 19 ++++++++++++++++++-
+ 4 files changed, 27 insertions(+), 41 deletions(-)
+
+diff --git a/include/rule.h b/include/rule.h
+index 00a1bac5a773..7625addfd919 100644
+--- a/include/rule.h
++++ b/include/rule.h
+@@ -733,6 +733,7 @@ extern struct cmd *cmd_alloc(enum cmd_ops op, enum cmd_obj 
obj,
+                            const struct handle *h, const struct location *loc,
+                            void *data);
+ extern void nft_cmd_expand(struct cmd *cmd);
++extern void nft_cmd_post_expand(struct cmd *cmd);
+ extern bool nft_cmd_collapse(struct list_head *cmds);
+ extern void nft_cmd_uncollapse(struct list_head *cmds);
+ extern struct cmd *cmd_alloc_obj_ct(enum cmd_ops op, int type,
+diff --git a/src/evaluate.c b/src/evaluate.c
+index c04cb91d3919..e2172569cdcc 100644
+--- a/src/evaluate.c
++++ b/src/evaluate.c
+@@ -4601,7 +4601,6 @@ static uint32_t str2hooknum(uint32_t family, const char 
*hook)
+ static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
+ {
+       struct table *table;
+-      struct rule *rule;
+ 
+       table = table_cache_find(&ctx->nft->cache.table_cache,
+                                ctx->cmd->handle.table.name,
+@@ -4657,11 +4656,6 @@ static int chain_evaluate(struct eval_ctx *ctx, struct 
chain *chain)
+               }
+       }
+ 
+-      list_for_each_entry(rule, &chain->rules, list) {
+-              handle_merge(&rule->handle, &chain->handle);
+-              if (rule_evaluate(ctx, rule, CMD_INVALID) < 0)
+-                      return -1;
+-      }
+       return 0;
+ }
+ 
+@@ -4729,11 +4723,6 @@ static int obj_evaluate(struct eval_ctx *ctx, struct 
obj *obj)
+ 
+ static int table_evaluate(struct eval_ctx *ctx, struct table *table)
+ {
+-      struct flowtable *ft;
+-      struct chain *chain;
+-      struct set *set;
+-      struct obj *obj;
+-
+       if (!table_cache_find(&ctx->nft->cache.table_cache,
+                             ctx->cmd->handle.table.name,
+                             ctx->cmd->handle.family)) {
+@@ -4746,34 +4735,6 @@ static int table_evaluate(struct eval_ctx *ctx, struct 
table *table)
+               }
+       }
+ 
+-      if (ctx->cmd->table == NULL)
+-              return 0;
+-
+-      ctx->table = table;
+-      list_for_each_entry(set, &table->sets, list) {
+-              expr_set_context(&ctx->ectx, NULL, 0);
+-              handle_merge(&set->handle, &table->handle);
+-              if (set_evaluate(ctx, set) < 0)
+-                      return -1;
+-      }
+-      list_for_each_entry(chain, &table->chains, list) {
+-              handle_merge(&chain->handle, &table->handle);
+-              ctx->cmd->handle.chain.location = chain->location;
+-              if (chain_evaluate(ctx, chain) < 0)
+-                      return -1;
+-      }
+-      list_for_each_entry(ft, &table->flowtables, list) {
+-              handle_merge(&ft->handle, &table->handle);
+-              if (flowtable_evaluate(ctx, ft) < 0)
+-                      return -1;
+-      }
+-      list_for_each_entry(obj, &table->objs, list) {
+-              handle_merge(&obj->handle, &table->handle);
+-              if (obj_evaluate(ctx, obj) < 0)
+-                      return -1;
+-      }
+-
+-      ctx->table = NULL;
+       return 0;
+ }
+ 
+diff --git a/src/libnftables.c b/src/libnftables.c
+index a376825d7309..fd4ee6988c27 100644
+--- a/src/libnftables.c
++++ b/src/libnftables.c
+@@ -520,6 +520,13 @@ static int nft_evaluate(struct nft_ctx *nft, struct 
list_head *msgs,
+       if (nft_cmd_collapse(cmds))
+               collapsed = true;
+ 
++      list_for_each_entry(cmd, cmds, list) {
++              if (cmd->op != CMD_ADD)
++                      continue;
++
++              nft_cmd_expand(cmd);
++      }
++
+       list_for_each_entry_safe(cmd, next, cmds, list) {
+               struct eval_ctx ectx = {
+                       .nft    = nft,
+@@ -543,7 +550,7 @@ static int nft_evaluate(struct nft_ctx *nft, struct 
list_head *msgs,
+               if (cmd->op != CMD_ADD)
+                       continue;
+ 
+-              nft_cmd_expand(cmd);
++              nft_cmd_post_expand(cmd);
+       }
+ 
+       return 0;
+diff --git a/src/rule.c b/src/rule.c
+index 323d89afd141..982160359aea 100644
+--- a/src/rule.c
++++ b/src/rule.c
+@@ -1318,8 +1318,9 @@ static void nft_cmd_expand_chain(struct chain *chain, 
struct list_head *new_cmds
+ 
+       list_for_each_entry_safe(rule, next, &chain->rules, list) {
+               list_del(&rule->list);
++              handle_merge(&rule->handle, &chain->handle);
+               memset(&h, 0, sizeof(h));
+-              handle_merge(&h, &rule->handle);
++              handle_merge(&h, &chain->handle);
+               if (chain->flags & CHAIN_F_BINDING) {
+                       rule->handle.chain_id = chain->handle.chain_id;
+                       rule->handle.chain.location = chain->location;
+@@ -1350,6 +1351,7 @@ void nft_cmd_expand(struct cmd *cmd)
+                       return;
+ 
+               list_for_each_entry(chain, &table->chains, list) {
++                      handle_merge(&chain->handle, &table->handle);
+                       memset(&h, 0, sizeof(h));
+                       handle_merge(&h, &chain->handle);
+                       h.chain_id = chain->handle.chain_id;
+@@ -1394,6 +1396,21 @@ void nft_cmd_expand(struct cmd *cmd)
+               nft_cmd_expand_chain(chain, &new_cmds);
+               list_splice(&new_cmds, &cmd->list);
+               break;
++      default:
++              break;
++      }
++}
++
++void nft_cmd_post_expand(struct cmd *cmd)
++{
++      struct list_head new_cmds;
++      struct set *set;
++      struct cmd *new;
++      struct handle h;
++
++      init_list_head(&new_cmds);
++
++      switch (cmd->obj) {
+       case CMD_OBJ_SET:
+       case CMD_OBJ_MAP:
+               set = cmd->set;
+-- 
+2.30.2
+

Reply via email to