[nft PATCH 0/3] Fix for exclusive start cond corner cases

[nft PATCH 0/3] Fix for exclusive start cond corner cases

[Phil Sutter]

Patch 2 fixes the issue xelra reported in [1], patch 1 fixes a related
issue and patch 3 extends scanner debugging output a bit which helped
when analyzing the bug.

[1] https://lore.kernel.org/netfilter-devel/VPAY0CnKhCaOKYE6oo_AU7sR5MaGIGzyPkXi00RlaucbyN78KYBqK73XfyJ-yyXQENIvvXeFJq_fNrYbJuW8LcqZvZyf4J3kr66jvsojxCU=@protonmail.com/

Phil Sutter (3):
  parser_bison: Fix for comment following exclusive start cond
  scanner: Fix delayed start condition pop logic
  scanner: Add start condition debugging

 src/scanner.l                                 | 21 +++++---
 .../dumps/exclusive_start_cond.json-nft       | 52 +++++++++++++++++++
 .../parsing/dumps/exclusive_start_cond.nft    |  2 +
 .../testcases/parsing/exclusive_start_cond    |  6 +++
 4 files changed, 73 insertions(+), 8 deletions(-)

-- 
2.54.0

[nft PATCH 1/3] parser_bison: Fix for comment following exclusive start cond

[Phil Sutter]

Fixed commit missed "comment" as yet another global scope keyword which
may immediately follow the limit statement, probably because rule
comments may not appear at arbitrary positions but strictly at rule end.
Therefore they are defined at an easily overlooked spot in parser_bison.y.

Fixes: dedd9f78393ec ("scanner: Accept all statements' first words in all scopes")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/scanner.l                                  |  2 +-
 .../dumps/exclusive_start_cond.json-nft        | 18 ++++++++++++++++++
 .../parsing/dumps/exclusive_start_cond.nft     |  1 +
 .../testcases/parsing/exclusive_start_cond     |  3 +++
 4 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/src/scanner.l b/src/scanner.l
index 37729ebc3cc55..b35dddd94eb4a 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -378,7 +378,7 @@ addrstring	({macaddr}|{ip4addr}|{ip6addr})
 
 "position"		{ return POSITION; }
 "index"			{ return INDEX; }
-"comment"		{ return COMMENT; }
+<*>"comment"		{ return COMMENT; }
 
 "constant"		{ return CONSTANT; }
 "interval"		{ return INTERVAL; }
diff --git a/tests/shell/testcases/parsing/dumps/exclusive_start_cond.json-nft b/tests/shell/testcases/parsing/dumps/exclusive_start_cond.json-nft
index 46ea3dcf24dd3..1de5007c6fc9d 100644
--- a/tests/shell/testcases/parsing/dumps/exclusive_start_cond.json-nft
+++ b/tests/shell/testcases/parsing/dumps/exclusive_start_cond.json-nft
@@ -2003,6 +2003,24 @@
         ]
       }
     },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "t",
+        "chain": "c",
+        "handle": 0,
+        "comment": "foobar",
+        "expr": [
+          {
+            "limit": {
+              "rate": 1,
+              "burst": 5,
+              "per": "second"
+            }
+          }
+        ]
+      }
+    },
     {
       "rule": {
         "family": "ip",
diff --git a/tests/shell/testcases/parsing/dumps/exclusive_start_cond.nft b/tests/shell/testcases/parsing/dumps/exclusive_start_cond.nft
index 7a7ce2141e057..1b9b369e67640 100644
--- a/tests/shell/testcases/parsing/dumps/exclusive_start_cond.nft
+++ b/tests/shell/testcases/parsing/dumps/exclusive_start_cond.nft
@@ -83,6 +83,7 @@ table ip t {
 		limit rate 1/second burst 5 packets fib daddr . iif check exists
 		limit rate 1/second burst 5 packets osf name "foo"
 		limit rate 1/second burst 5 packets ipsec in spi 0
+		limit rate 1/second burst 5 packets comment "foobar"
 		meta l4proto 6 limit rate 1/second burst 5 packets tproxy to 0.0.0.1
 		meta l4proto 17 limit rate 1/second burst 5 packets vxlan vni 0
 		meta l4proto 17 limit rate 1/second burst 5 packets geneve vni 0
diff --git a/tests/shell/testcases/parsing/exclusive_start_cond b/tests/shell/testcases/parsing/exclusive_start_cond
index f769e7dd1432a..1fce6826f7cca 100755
--- a/tests/shell/testcases/parsing/exclusive_start_cond
+++ b/tests/shell/testcases/parsing/exclusive_start_cond
@@ -95,6 +95,9 @@ stmts+=("osf name foo")
 # xfrm_expr
 stmts+=("ipsec in spi 0")
 
+# comment_spec
+stmts+=("comment foobar")
+
 $NFT -f - <<EOF
 table t {
 	flowtable ft {
-- 
2.54.0

[nft PATCH 2/3] scanner: Fix delayed start condition pop logic

[Phil Sutter]

If scanner_pop_start_cond() is called and the passed start condition is
currently active, it is supposed to pop it and all delayed states
identified by a zero field value in state->startcond_active. The counter
state->flex_state_pop reflects the number of delayed start conditions
waiting to be popped.

The implementation mixed up popping of the currently active state with
delayed ones though and therefore decremented flex_state_pop for the
currently active (i.e., not delayed) state as well. As a consequence,
the first delayed state remained active after the function returned,
causing problems in corner-cases.

Reported-by: xelra <xelra@protonmail.com>
Fixes: 8623772af0610 ("scanner: don't pop active flex scanner scope")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/scanner.l                                 | 11 +++---
 .../dumps/exclusive_start_cond.json-nft       | 34 +++++++++++++++++++
 .../parsing/dumps/exclusive_start_cond.nft    |  1 +
 .../testcases/parsing/exclusive_start_cond    |  3 ++
 4 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/src/scanner.l b/src/scanner.l
index b35dddd94eb4a..aa83d8590f5e1 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -1404,17 +1404,14 @@ void scanner_pop_start_cond(void *scanner, enum startcond_type t)
 		return; /* Can't pop just yet! */
 	}
 
-	while (state->flex_state_pop) {
+	t = state->startcond_type = yy_top_state(scanner);
+	yy_pop_state(scanner);
+
+	while (state->flex_state_pop && !state->startcond_active[t]) {
 		state->flex_state_pop--;
 		state->startcond_type = yy_top_state(scanner);
 		yy_pop_state(scanner);
 
 		t = state->startcond_type;
-		if (state->startcond_active[t])
-			return;
 	}
-
-	state->startcond_type = yy_top_state(scanner);
-
-	yy_pop_state(scanner);
 }
diff --git a/tests/shell/testcases/parsing/dumps/exclusive_start_cond.json-nft b/tests/shell/testcases/parsing/dumps/exclusive_start_cond.json-nft
index 1de5007c6fc9d..092a5278cf708 100644
--- a/tests/shell/testcases/parsing/dumps/exclusive_start_cond.json-nft
+++ b/tests/shell/testcases/parsing/dumps/exclusive_start_cond.json-nft
@@ -2021,6 +2021,40 @@
         ]
       }
     },
+    {
+      "rule": {
+        "family": "ip",
+        "table": "t",
+        "chain": "c",
+        "handle": 0,
+        "comment": "x",
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "meta": {
+                  "key": "l4proto"
+                }
+              },
+              "right": 6
+            }
+          },
+          {
+            "limit": {
+              "rate": 1,
+              "burst": 5,
+              "per": "second"
+            }
+          },
+          {
+            "reject": {
+              "type": "tcp reset"
+            }
+          }
+        ]
+      }
+    },
     {
       "rule": {
         "family": "ip",
diff --git a/tests/shell/testcases/parsing/dumps/exclusive_start_cond.nft b/tests/shell/testcases/parsing/dumps/exclusive_start_cond.nft
index 1b9b369e67640..a1a6e44faf6ed 100644
--- a/tests/shell/testcases/parsing/dumps/exclusive_start_cond.nft
+++ b/tests/shell/testcases/parsing/dumps/exclusive_start_cond.nft
@@ -84,6 +84,7 @@ table ip t {
 		limit rate 1/second burst 5 packets osf name "foo"
 		limit rate 1/second burst 5 packets ipsec in spi 0
 		limit rate 1/second burst 5 packets comment "foobar"
+		meta l4proto 6 limit rate 1/second burst 5 packets reject with tcp reset comment "x"
 		meta l4proto 6 limit rate 1/second burst 5 packets tproxy to 0.0.0.1
 		meta l4proto 17 limit rate 1/second burst 5 packets vxlan vni 0
 		meta l4proto 17 limit rate 1/second burst 5 packets geneve vni 0
diff --git a/tests/shell/testcases/parsing/exclusive_start_cond b/tests/shell/testcases/parsing/exclusive_start_cond
index 1fce6826f7cca..b4b34b23d428e 100755
--- a/tests/shell/testcases/parsing/exclusive_start_cond
+++ b/tests/shell/testcases/parsing/exclusive_start_cond
@@ -98,6 +98,9 @@ stmts+=("ipsec in spi 0")
 # comment_spec
 stmts+=("comment foobar")
 
+# this revealed a bug in scanner_pop_start_cond() code
+stmts+=("reject with tcp reset comment x")
+
 $NFT -f - <<EOF
 table t {
 	flowtable ft {
-- 
2.54.0

Re: [nft PATCH 2/3] scanner: Fix delayed start condition pop logic

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> If scanner_pop_start_cond() is called and the passed start condition is
> currently active, it is supposed to pop it and all delayed states
> identified by a zero field value in state->startcond_active. The counter
> state->flex_state_pop reflects the number of delayed start conditions
> waiting to be popped.
> 
> The implementation mixed up popping of the currently active state with
> delayed ones though and therefore decremented flex_state_pop for the
> currently active (i.e., not delayed) state as well. As a consequence,
> the first delayed state remained active after the function returned,
> causing problems in corner-cases.

Right, it doesn't matter for inclusive states.

Thanks for fixing this up.

[nft PATCH 3/3] scanner: Add start condition debugging

[Phil Sutter]

Report start condition push/pop actions when called with
--debug=scanner.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/scanner.l | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/scanner.l b/src/scanner.l
index aa83d8590f5e1..a4f9a16c5523a 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -1383,10 +1383,13 @@ void scanner_destroy(struct nft_ctx *nft)
 	yylex_destroy(nft->scanner);
 }
 
+#define sc_dbg(...) if (nft_get_debug(scanner)) fprintf(stderr, __VA_ARGS__)
+
 static void scanner_push_start_cond(void *scanner, enum startcond_type type)
 {
 	struct parser_state *state = yyget_extra(scanner);
 
+	sc_dbg("--pushing start condition %d\n", type);
 	state->startcond_type = type;
 	state->startcond_active[type]++;
 
@@ -1401,14 +1404,19 @@ void scanner_pop_start_cond(void *scanner, enum startcond_type t)
 
 	if (state->startcond_type != t) {
 		state->flex_state_pop++;
+		sc_dbg("--delay popping start condition %d\n", t);
 		return; /* Can't pop just yet! */
 	}
 
+	sc_dbg("--popping start condition %d\n", state->startcond_type);
+
 	t = state->startcond_type = yy_top_state(scanner);
 	yy_pop_state(scanner);
 
 	while (state->flex_state_pop && !state->startcond_active[t]) {
 		state->flex_state_pop--;
+		sc_dbg("--popping delayed start condition %d\n",
+		       state->startcond_type);
 		state->startcond_type = yy_top_state(scanner);
 		yy_pop_state(scanner);
 
-- 
2.54.0

Re: [nft PATCH 0/3] Fix for exclusive start cond corner cases

[Phil Sutter]

On Thu, Nov 05, 2026 at 06:37:10PM +0100, Phil Sutter wrote:
> Patch 2 fixes the issue xelra reported in [1], patch 1 fixes a related
> issue and patch 3 extends scanner debugging output a bit which helped
> when analyzing the bug.
> 
> [1] https://lore.kernel.org/netfilter-devel/VPAY0CnKhCaOKYE6oo_AU7sR5MaGIGzyPkXi00RlaucbyN78KYBqK73XfyJ-yyXQENIvvXeFJq_fNrYbJuW8LcqZvZyf4J3kr66jvsojxCU=@protonmail.com/
> 
> Phil Sutter (3):
>   parser_bison: Fix for comment following exclusive start cond
>   scanner: Fix delayed start condition pop logic
>   scanner: Add start condition debugging

Series applied.