netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH nft 0/4] nft tunnel mode parser/eval fixes
@ 2025-10-16 14:59 Florian Westphal
  2025-10-16 14:59 ` [PATCH nft 1/4] evaluate: tunnel: don't assume src is set Florian Westphal
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Florian Westphal @ 2025-10-16 14:59 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

This series addresses a few bugs found with afl fuzzer, see individual
patches for details.

Florian Westphal (4):
  evaluate: tunnel: don't assume src is set
  src: tunnel src/dst must be a symbolic expression
  src: parser_bison: prevent multiple ip daddr/saddr definitions
  evaluate: reject tunnel section if another one is already present

 src/evaluate.c                                | 29 +++++++--
 src/parser_bison.y                            | 63 ++++++++++++++++---
 .../nft-f/empty_geneve_definition_crash       |  4 ++
 .../bogons/nft-f/tunnel_in_tunnel_crash       | 10 +++
 .../bogons/nft-f/tunnel_with_anon_set_assert  |  9 +++
 .../bogons/nft-f/tunnel_with_garbage_dst      |  5 ++
 6 files changed, 104 insertions(+), 16 deletions(-)
 create mode 100644 tests/shell/testcases/bogons/nft-f/empty_geneve_definition_crash
 create mode 100644 tests/shell/testcases/bogons/nft-f/tunnel_in_tunnel_crash
 create mode 100644 tests/shell/testcases/bogons/nft-f/tunnel_with_anon_set_assert
 create mode 100644 tests/shell/testcases/bogons/nft-f/tunnel_with_garbage_dst

-- 
2.51.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH nft 1/4] evaluate: tunnel: don't assume src is set
  2025-10-16 14:59 [PATCH nft 0/4] nft tunnel mode parser/eval fixes Florian Westphal
@ 2025-10-16 14:59 ` Florian Westphal
  2025-10-16 23:37   ` Fernando Fernandez Mancera
  2025-10-16 14:59 ` [PATCH nft 2/4] src: tunnel src/dst must be a symbolic expression Florian Westphal
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Florian Westphal @ 2025-10-16 14:59 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

Included bogon crashes, after fix:

empty_geneve_definition_crash:2:9-16: Error: Could not process rule: Invalid argument

Since this feature is undocumented (hint, hint) I don't know
if there are cases where ip daddr can be elided.

If not, a followup patch should reject empty dst upfront
so users get a more verbose error message.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/evaluate.c                                           | 9 +++++----
 .../testcases/bogons/nft-f/empty_geneve_definition_crash | 4 ++++
 2 files changed, 9 insertions(+), 4 deletions(-)
 create mode 100644 tests/shell/testcases/bogons/nft-f/empty_geneve_definition_crash

diff --git a/src/evaluate.c b/src/evaluate.c
index 0c7d90f8f43b..ac482c83cce2 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -5865,11 +5865,12 @@ static int tunnel_evaluate(struct eval_ctx *ctx, struct obj *obj)
 				 obj->tunnel.dst->dtype->size);
 		if (expr_evaluate(ctx, &obj->tunnel.dst) < 0)
 			return -1;
-	}
 
-	if (obj->tunnel.src->dtype != obj->tunnel.dst->dtype)
-		return __stmt_binary_error(ctx, &obj->location, NULL,
-					  "specify either ip or ip6 for address");
+		if (obj->tunnel.src &&
+		    obj->tunnel.src->dtype != obj->tunnel.dst->dtype)
+			return __stmt_binary_error(ctx, &obj->location, NULL,
+						  "specify either ip or ip6 for address");
+	}
 
 	return 0;
 }
diff --git a/tests/shell/testcases/bogons/nft-f/empty_geneve_definition_crash b/tests/shell/testcases/bogons/nft-f/empty_geneve_definition_crash
new file mode 100644
index 000000000000..d1bc76c56bd5
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/empty_geneve_definition_crash
@@ -0,0 +1,4 @@
+table netdev x {
+	tunnel geneve-t {
+	}
+}
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH nft 2/4] src: tunnel src/dst must be a symbolic expression
  2025-10-16 14:59 [PATCH nft 0/4] nft tunnel mode parser/eval fixes Florian Westphal
  2025-10-16 14:59 ` [PATCH nft 1/4] evaluate: tunnel: don't assume src is set Florian Westphal
@ 2025-10-16 14:59 ` Florian Westphal
  2025-10-16 23:39   ` Fernando Fernandez Mancera
  2025-10-16 14:59 ` [PATCH nft 3/4] src: parser_bison: prevent multiple ip daddr/saddr definitions Florian Westphal
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Florian Westphal @ 2025-10-16 14:59 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

Included bogons crash with segfault and assertion.  After fix:

tunnel_with_garbage_dst:3:12-14: Error: syntax error, unexpected tcp, expecting string or quoted string or string with a trailing asterisk or '$'
  ip saddr tcp dport { }
           ^^^
The parser change restricts the grammar to no longer allow this,
we would crash here because we enter payload evaluation path that
tries to insert a dependency into the rule, but we don't have one
(ctx->rule and ctx->stmt are NULL as expected here).

The eval stage change makes sure we will reject non-value symbols:

tunnel_with_anon_set_assert:1:12-31: Error: must be a value, not set
define s = { 1.2.3.4, 5.6.7.8 }
           ^^^^^^^^^^^^^^^^^^^^

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/evaluate.c                                | 20 +++++++++++++++++--
 src/parser_bison.y                            |  8 ++++----
 .../bogons/nft-f/tunnel_with_anon_set_assert  |  8 ++++++++
 .../bogons/nft-f/tunnel_with_garbage_dst      |  5 +++++
 4 files changed, 35 insertions(+), 6 deletions(-)
 create mode 100644 tests/shell/testcases/bogons/nft-f/tunnel_with_anon_set_assert
 create mode 100644 tests/shell/testcases/bogons/nft-f/tunnel_with_garbage_dst

diff --git a/src/evaluate.c b/src/evaluate.c
index ac482c83cce2..a5cc41819198 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -5851,19 +5851,35 @@ static int ct_timeout_evaluate(struct eval_ctx *ctx, struct obj *obj)
 	return 0;
 }
 
+static int tunnel_evaluate_addr(struct eval_ctx *ctx, struct expr **exprp)
+{
+	struct expr *e;
+	int ret;
+
+	ret = expr_evaluate(ctx, exprp);
+	if (ret < 0)
+		return ret;
+
+	e = *exprp;
+	if (e->etype != EXPR_VALUE)
+		return expr_error(ctx->msgs, e, "must be a value, not %s", expr_name(e));
+
+	return 0;
+}
+
 static int tunnel_evaluate(struct eval_ctx *ctx, struct obj *obj)
 {
 	if (obj->tunnel.src) {
 		expr_set_context(&ctx->ectx, obj->tunnel.src->dtype,
 				 obj->tunnel.src->dtype->size);
-		if (expr_evaluate(ctx, &obj->tunnel.src) < 0)
+		if (tunnel_evaluate_addr(ctx, &obj->tunnel.src) < 0)
 			return -1;
 	}
 
 	if (obj->tunnel.dst) {
 		expr_set_context(&ctx->ectx, obj->tunnel.dst->dtype,
 				 obj->tunnel.dst->dtype->size);
-		if (expr_evaluate(ctx, &obj->tunnel.dst) < 0)
+		if (tunnel_evaluate_addr(ctx, &obj->tunnel.dst) < 0)
 			return -1;
 
 		if (obj->tunnel.src &&
diff --git a/src/parser_bison.y b/src/parser_bison.y
index 100a5c871e61..b63c7df18a35 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -5068,22 +5068,22 @@ tunnel_config		:	ID	NUM
 			{
 				$<obj>0->tunnel.id = $2;
 			}
-			|	IP	SADDR	expr	close_scope_ip
+			|	IP	SADDR	symbol_expr	close_scope_ip
 			{
 				$<obj>0->tunnel.src = $3;
 				datatype_set($3, &ipaddr_type);
 			}
-			|	IP	DADDR	expr	close_scope_ip
+			|	IP	DADDR	symbol_expr	close_scope_ip
 			{
 				$<obj>0->tunnel.dst = $3;
 				datatype_set($3, &ipaddr_type);
 			}
-			|	IP6	SADDR	expr	close_scope_ip6
+			|	IP6	SADDR	symbol_expr	close_scope_ip6
 			{
 				$<obj>0->tunnel.src = $3;
 				datatype_set($3, &ip6addr_type);
 			}
-			|	IP6	DADDR	expr	close_scope_ip6
+			|	IP6	DADDR	symbol_expr	close_scope_ip6
 			{
 				$<obj>0->tunnel.dst = $3;
 				datatype_set($3, &ip6addr_type);
diff --git a/tests/shell/testcases/bogons/nft-f/tunnel_with_anon_set_assert b/tests/shell/testcases/bogons/nft-f/tunnel_with_anon_set_assert
new file mode 100644
index 000000000000..6f7b212aefef
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/tunnel_with_anon_set_assert
@@ -0,0 +1,8 @@
+define s = { 1.2.3.4, 5.6.7.8 }
+
+table netdev x {
+	tunnel t {
+		ip saddr $s
+	}
+	}
+
diff --git a/tests/shell/testcases/bogons/nft-f/tunnel_with_garbage_dst b/tests/shell/testcases/bogons/nft-f/tunnel_with_garbage_dst
new file mode 100644
index 000000000000..85eb992cec16
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/tunnel_with_garbage_dst
@@ -0,0 +1,5 @@
+table netdev x {
+	tunnel t {
+		ip saddr tcp dport { }
+	}
+}
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH nft 3/4] src: parser_bison: prevent multiple ip daddr/saddr definitions
  2025-10-16 14:59 [PATCH nft 0/4] nft tunnel mode parser/eval fixes Florian Westphal
  2025-10-16 14:59 ` [PATCH nft 1/4] evaluate: tunnel: don't assume src is set Florian Westphal
  2025-10-16 14:59 ` [PATCH nft 2/4] src: tunnel src/dst must be a symbolic expression Florian Westphal
@ 2025-10-16 14:59 ` Florian Westphal
  2025-10-16 23:41   ` Fernando Fernandez Mancera
  2025-10-16 14:59 ` [PATCH nft 4/4] evaluate: reject tunnel section if another one is already present Florian Westphal
  2025-10-16 23:46 ` [PATCH nft 0/4] nft tunnel mode parser/eval fixes Fernando Fernandez Mancera
  4 siblings, 1 reply; 11+ messages in thread
From: Florian Westphal @ 2025-10-16 14:59 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

minor change to the bogon makes it assert because symbolic expression
will have wrong refcount (2) at scope teardown.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/parser_bison.y                              | 17 +++++++++++++++++
 .../bogons/nft-f/tunnel_with_anon_set_assert    |  1 +
 2 files changed, 18 insertions(+)

diff --git a/src/parser_bison.y b/src/parser_bison.y
index b63c7df18a35..4e028d31c165 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -5070,21 +5070,38 @@ tunnel_config		:	ID	NUM
 			}
 			|	IP	SADDR	symbol_expr	close_scope_ip
 			{
+				if (already_set($<obj>0->tunnel.src, &@3, state)) {
+					expr_free($3);
+					YYERROR;
+				}
+
 				$<obj>0->tunnel.src = $3;
 				datatype_set($3, &ipaddr_type);
 			}
 			|	IP	DADDR	symbol_expr	close_scope_ip
 			{
+				if (already_set($<obj>0->tunnel.dst, &@3, state)) {
+					expr_free($3);
+					YYERROR;
+				}
 				$<obj>0->tunnel.dst = $3;
 				datatype_set($3, &ipaddr_type);
 			}
 			|	IP6	SADDR	symbol_expr	close_scope_ip6
 			{
+				if (already_set($<obj>0->tunnel.src, &@3, state)) {
+					expr_free($3);
+					YYERROR;
+				}
 				$<obj>0->tunnel.src = $3;
 				datatype_set($3, &ip6addr_type);
 			}
 			|	IP6	DADDR	symbol_expr	close_scope_ip6
 			{
+				if (already_set($<obj>0->tunnel.dst, &@3, state)) {
+					expr_free($3);
+					YYERROR;
+				}
 				$<obj>0->tunnel.dst = $3;
 				datatype_set($3, &ip6addr_type);
 			}
diff --git a/tests/shell/testcases/bogons/nft-f/tunnel_with_anon_set_assert b/tests/shell/testcases/bogons/nft-f/tunnel_with_anon_set_assert
index 6f7b212aefef..d02568944301 100644
--- a/tests/shell/testcases/bogons/nft-f/tunnel_with_anon_set_assert
+++ b/tests/shell/testcases/bogons/nft-f/tunnel_with_anon_set_assert
@@ -3,6 +3,7 @@ define s = { 1.2.3.4, 5.6.7.8 }
 table netdev x {
 	tunnel t {
 		ip saddr $s
+		ip saddr $s
 	}
 	}
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH nft 4/4] evaluate: reject tunnel section if another one is already present
  2025-10-16 14:59 [PATCH nft 0/4] nft tunnel mode parser/eval fixes Florian Westphal
                   ` (2 preceding siblings ...)
  2025-10-16 14:59 ` [PATCH nft 3/4] src: parser_bison: prevent multiple ip daddr/saddr definitions Florian Westphal
@ 2025-10-16 14:59 ` Florian Westphal
  2025-10-16 23:44   ` Fernando Fernandez Mancera
  2025-10-16 23:46 ` [PATCH nft 0/4] nft tunnel mode parser/eval fixes Fernando Fernandez Mancera
  4 siblings, 1 reply; 11+ messages in thread
From: Florian Westphal @ 2025-10-16 14:59 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal

Included bogon causes a crash because the list head isn't initialised
due to tunnel->type == VXLAN.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/parser_bison.y                            | 38 ++++++++++++++++---
 .../bogons/nft-f/tunnel_in_tunnel_crash       | 10 +++++
 2 files changed, 42 insertions(+), 6 deletions(-)
 create mode 100644 tests/shell/testcases/bogons/nft-f/tunnel_in_tunnel_crash

diff --git a/src/parser_bison.y b/src/parser_bison.y
index 4e028d31c165..3c21c7584d01 100644
--- a/src/parser_bison.y
+++ b/src/parser_bison.y
@@ -144,6 +144,19 @@ static bool already_set(const void *attr, const struct location *loc,
 	return true;
 }
 
+static bool tunnel_set_type(const struct location *loc,
+			    struct obj *obj, enum tunnel_type type, const char *name,
+			    struct parser_state *state)
+{
+	if (obj->tunnel.type) {
+		erec_queue(error(loc, "Cannot create new %s section inside another tunnel", name), state->msgs);
+		return false;
+	}
+
+	obj->tunnel.type = type;
+	return true;
+}
+
 static struct expr *ifname_expr_alloc(const struct location *location,
 				      struct list_head *queue,
 				      const char *name)
@@ -4980,11 +4993,15 @@ erspan_block		:	/* empty */	{ $$ = $<obj>-1; }
 erspan_block_alloc	:	/* empty */
 			{
 				$$ = $<obj>-1;
+
+				if (!tunnel_set_type(&$$->location, $$, TUNNEL_ERSPAN, "erspan", state))
+					YYERROR;
 			}
 			;
 
 erspan_config		:	HDRVERSION	NUM
 			{
+				assert($<obj>0->tunnel.type == TUNNEL_ERSPAN);
 				$<obj>0->tunnel.erspan.version = $2;
 			}
 			|	INDEX		NUM
@@ -5017,6 +5034,10 @@ geneve_block		:	/* empty */	{ $$ = $<obj>-1; }
 geneve_block_alloc	:	/* empty */
 			{
 				$$ = $<obj>-1;
+				if (!tunnel_set_type(&$$->location, $$, TUNNEL_GENEVE, "geneve", state))
+					YYERROR;
+
+				init_list_head(&$$->tunnel.geneve_opts);
 			}
 			;
 
@@ -5024,6 +5045,8 @@ geneve_config		:	CLASS	NUM	OPTTYPE	NUM	DATA	string
 			{
 				struct tunnel_geneve *geneve;
 
+				assert($<obj>0->tunnel.type == TUNNEL_GENEVE);
+
 				geneve = xmalloc(sizeof(struct tunnel_geneve));
 				geneve->geneve_class = $2;
 				geneve->type = $4;
@@ -5034,10 +5057,6 @@ geneve_config		:	CLASS	NUM	OPTTYPE	NUM	DATA	string
 					YYERROR;
 				}
 
-				if (!$<obj>0->tunnel.type) {
-					$<obj>0->tunnel.type = TUNNEL_GENEVE;
-					init_list_head(&$<obj>0->tunnel.geneve_opts);
-				}
 				list_add_tail(&geneve->list, &$<obj>0->tunnel.geneve_opts);
 				free_const($6);
 			}
@@ -5055,11 +5074,15 @@ vxlan_block		:	/* empty */	{ $$ = $<obj>-1; }
 vxlan_block_alloc	:	/* empty */
 			{
 				$$ = $<obj>-1;
+
+				if (!tunnel_set_type(&$$->location, $$, TUNNEL_VXLAN, "vxlan", state))
+					YYERROR;
 			}
 			;
 
 vxlan_config		:	GBP	NUM
 			{
+				assert($<obj>0->tunnel.type == TUNNEL_VXLAN);
 				$<obj>0->tunnel.vxlan.gbp = $2;
 			}
 			;
@@ -5123,13 +5146,16 @@ tunnel_config		:	ID	NUM
 			}
 			|	ERSPAN	erspan_block_alloc '{' erspan_block '}'
 			{
-				$<obj>0->tunnel.type = TUNNEL_ERSPAN;
+				$2->location = @2;
 			}
 			|	VXLAN	vxlan_block_alloc '{' vxlan_block '}'
 			{
-				$<obj>0->tunnel.type = TUNNEL_VXLAN;
+				$2->location = @2;
 			}
 			|	GENEVE	geneve_block_alloc '{' geneve_block '}'
+			{
+				$2->location = @2;
+			}
 			;
 
 tunnel_block		:	/* empty */	{ $$ = $<obj>-1; }
diff --git a/tests/shell/testcases/bogons/nft-f/tunnel_in_tunnel_crash b/tests/shell/testcases/bogons/nft-f/tunnel_in_tunnel_crash
new file mode 100644
index 000000000000..9f029807f521
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/tunnel_in_tunnel_crash
@@ -0,0 +1,10 @@
+table netdev x {
+	tunnel geneve-t {
+		vxlan {
+			gbp 200
+		}
+		geneve {
+			class 0x1 opt-type 0x1 data "0x12345678"
+		}
+	}
+
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH nft 1/4] evaluate: tunnel: don't assume src is set
  2025-10-16 14:59 ` [PATCH nft 1/4] evaluate: tunnel: don't assume src is set Florian Westphal
@ 2025-10-16 23:37   ` Fernando Fernandez Mancera
  0 siblings, 0 replies; 11+ messages in thread
From: Fernando Fernandez Mancera @ 2025-10-16 23:37 UTC (permalink / raw)
  To: Florian Westphal, netfilter-devel


On 10/16/25 4:59 PM, Florian Westphal wrote:
> Included bogon crashes, after fix:
> 
> empty_geneve_definition_crash:2:9-16: Error: Could not process rule: Invalid argument
> 
> Since this feature is undocumented (hint, hint) I don't know
> if there are cases where ip daddr can be elided.
> 

Ugh, sorry. That is my fault, I am going to document this properly.

> If not, a followup patch should reject empty dst upfront
> so users get a more verbose error message.
> 

dst cannot be empty so it should be rejected properly providing a good 
error message. Let me handle that on a follow up patch.

> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>   src/evaluate.c                                           | 9 +++++----
>   .../testcases/bogons/nft-f/empty_geneve_definition_crash | 4 ++++
>   2 files changed, 9 insertions(+), 4 deletions(-)
>   create mode 100644 tests/shell/testcases/bogons/nft-f/empty_geneve_definition_crash
> 
> diff --git a/src/evaluate.c b/src/evaluate.c
> index 0c7d90f8f43b..ac482c83cce2 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -5865,11 +5865,12 @@ static int tunnel_evaluate(struct eval_ctx *ctx, struct obj *obj)
>   				 obj->tunnel.dst->dtype->size);
>   		if (expr_evaluate(ctx, &obj->tunnel.dst) < 0)
>   			return -1;
> -	}
>   
> -	if (obj->tunnel.src->dtype != obj->tunnel.dst->dtype)
> -		return __stmt_binary_error(ctx, &obj->location, NULL,
> -					  "specify either ip or ip6 for address");
> +		if (obj->tunnel.src &&
> +		    obj->tunnel.src->dtype != obj->tunnel.dst->dtype)
> +			return __stmt_binary_error(ctx, &obj->location, NULL,
> +						  "specify either ip or ip6 for address");
> +	}
>   
>   	return 0;
>   }
> diff --git a/tests/shell/testcases/bogons/nft-f/empty_geneve_definition_crash b/tests/shell/testcases/bogons/nft-f/empty_geneve_definition_crash
> new file mode 100644
> index 000000000000..d1bc76c56bd5
> --- /dev/null
> +++ b/tests/shell/testcases/bogons/nft-f/empty_geneve_definition_crash
> @@ -0,0 +1,4 @@
> +table netdev x {
> +	tunnel geneve-t {
> +	}
> +}

Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH nft 2/4] src: tunnel src/dst must be a symbolic expression
  2025-10-16 14:59 ` [PATCH nft 2/4] src: tunnel src/dst must be a symbolic expression Florian Westphal
@ 2025-10-16 23:39   ` Fernando Fernandez Mancera
  0 siblings, 0 replies; 11+ messages in thread
From: Fernando Fernandez Mancera @ 2025-10-16 23:39 UTC (permalink / raw)
  To: Florian Westphal, netfilter-devel



On 10/16/25 4:59 PM, Florian Westphal wrote:
> Included bogons crash with segfault and assertion.  After fix:
> 
> tunnel_with_garbage_dst:3:12-14: Error: syntax error, unexpected tcp, expecting string or quoted string or string with a trailing asterisk or '$'
>    ip saddr tcp dport { }
>             ^^^
> The parser change restricts the grammar to no longer allow this,
> we would crash here because we enter payload evaluation path that
> tries to insert a dependency into the rule, but we don't have one
> (ctx->rule and ctx->stmt are NULL as expected here).
> 
> The eval stage change makes sure we will reject non-value symbols:
> 
> tunnel_with_anon_set_assert:1:12-31: Error: must be a value, not set
> define s = { 1.2.3.4, 5.6.7.8 }
>             ^^^^^^^^^^^^^^^^^^^^
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>   src/evaluate.c                                | 20 +++++++++++++++++--
>   src/parser_bison.y                            |  8 ++++----
>   .../bogons/nft-f/tunnel_with_anon_set_assert  |  8 ++++++++
>   .../bogons/nft-f/tunnel_with_garbage_dst      |  5 +++++
>   4 files changed, 35 insertions(+), 6 deletions(-)
>   create mode 100644 tests/shell/testcases/bogons/nft-f/tunnel_with_anon_set_assert
>   create mode 100644 tests/shell/testcases/bogons/nft-f/tunnel_with_garbage_dst
> 

Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH nft 3/4] src: parser_bison: prevent multiple ip daddr/saddr definitions
  2025-10-16 14:59 ` [PATCH nft 3/4] src: parser_bison: prevent multiple ip daddr/saddr definitions Florian Westphal
@ 2025-10-16 23:41   ` Fernando Fernandez Mancera
  0 siblings, 0 replies; 11+ messages in thread
From: Fernando Fernandez Mancera @ 2025-10-16 23:41 UTC (permalink / raw)
  To: Florian Westphal, netfilter-devel



On 10/16/25 4:59 PM, Florian Westphal wrote:
> minor change to the bogon makes it assert because symbolic expression
> will have wrong refcount (2) at scope teardown.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>   src/parser_bison.y                              | 17 +++++++++++++++++
>   .../bogons/nft-f/tunnel_with_anon_set_assert    |  1 +
>   2 files changed, 18 insertions(+)
> 

Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH nft 4/4] evaluate: reject tunnel section if another one is already present
  2025-10-16 14:59 ` [PATCH nft 4/4] evaluate: reject tunnel section if another one is already present Florian Westphal
@ 2025-10-16 23:44   ` Fernando Fernandez Mancera
  0 siblings, 0 replies; 11+ messages in thread
From: Fernando Fernandez Mancera @ 2025-10-16 23:44 UTC (permalink / raw)
  To: Florian Westphal, netfilter-devel



On 10/16/25 4:59 PM, Florian Westphal wrote:
> Included bogon causes a crash because the list head isn't initialised
> due to tunnel->type == VXLAN.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>   src/parser_bison.y                            | 38 ++++++++++++++++---
>   .../bogons/nft-f/tunnel_in_tunnel_crash       | 10 +++++
>   2 files changed, 42 insertions(+), 6 deletions(-)
>   create mode 100644 tests/shell/testcases/bogons/nft-f/tunnel_in_tunnel_crash

Reviewed-by: Fernando Fernandez Mancera <fmancera@suse.de>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH nft 0/4] nft tunnel mode parser/eval fixes
  2025-10-16 14:59 [PATCH nft 0/4] nft tunnel mode parser/eval fixes Florian Westphal
                   ` (3 preceding siblings ...)
  2025-10-16 14:59 ` [PATCH nft 4/4] evaluate: reject tunnel section if another one is already present Florian Westphal
@ 2025-10-16 23:46 ` Fernando Fernandez Mancera
  2025-10-17  9:39   ` Florian Westphal
  4 siblings, 1 reply; 11+ messages in thread
From: Fernando Fernandez Mancera @ 2025-10-16 23:46 UTC (permalink / raw)
  To: Florian Westphal, netfilter-devel



On 10/16/25 4:59 PM, Florian Westphal wrote:
> This series addresses a few bugs found with afl fuzzer, see individual
> patches for details.
> 

Sorry for making you waste time here. I will follow up with the 
improvements mentioned on the other emails.

In addition, I want to set up afl fuzzer too :)

Thank you!

> Florian Westphal (4):
>    evaluate: tunnel: don't assume src is set
>    src: tunnel src/dst must be a symbolic expression
>    src: parser_bison: prevent multiple ip daddr/saddr definitions
>    evaluate: reject tunnel section if another one is already present
> 
>   src/evaluate.c                                | 29 +++++++--
>   src/parser_bison.y                            | 63 ++++++++++++++++---
>   .../nft-f/empty_geneve_definition_crash       |  4 ++
>   .../bogons/nft-f/tunnel_in_tunnel_crash       | 10 +++
>   .../bogons/nft-f/tunnel_with_anon_set_assert  |  9 +++
>   .../bogons/nft-f/tunnel_with_garbage_dst      |  5 ++
>   6 files changed, 104 insertions(+), 16 deletions(-)
>   create mode 100644 tests/shell/testcases/bogons/nft-f/empty_geneve_definition_crash
>   create mode 100644 tests/shell/testcases/bogons/nft-f/tunnel_in_tunnel_crash
>   create mode 100644 tests/shell/testcases/bogons/nft-f/tunnel_with_anon_set_assert
>   create mode 100644 tests/shell/testcases/bogons/nft-f/tunnel_with_garbage_dst
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH nft 0/4] nft tunnel mode parser/eval fixes
  2025-10-16 23:46 ` [PATCH nft 0/4] nft tunnel mode parser/eval fixes Fernando Fernandez Mancera
@ 2025-10-17  9:39   ` Florian Westphal
  0 siblings, 0 replies; 11+ messages in thread
From: Florian Westphal @ 2025-10-17  9:39 UTC (permalink / raw)
  To: Fernando Fernandez Mancera; +Cc: netfilter-devel

Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> On 10/16/25 4:59 PM, Florian Westphal wrote:
> > This series addresses a few bugs found with afl fuzzer, see individual
> > patches for details.
> > 
> 
> Sorry for making you waste time here. I will follow up with the 
> improvements mentioned on the other emails.

No time wasted.  I rebased and retested the fuzzer patch
for nftables so it was good it found some stuff :-)

Thanks for reviewing, I've pushed the patches to nftables.git.

Also thanks for following up with better error messages.

> In addition, I want to set up afl fuzzer too :)

Great, I will send the patch in a few minutes.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-10-17  9:39 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-16 14:59 [PATCH nft 0/4] nft tunnel mode parser/eval fixes Florian Westphal
2025-10-16 14:59 ` [PATCH nft 1/4] evaluate: tunnel: don't assume src is set Florian Westphal
2025-10-16 23:37   ` Fernando Fernandez Mancera
2025-10-16 14:59 ` [PATCH nft 2/4] src: tunnel src/dst must be a symbolic expression Florian Westphal
2025-10-16 23:39   ` Fernando Fernandez Mancera
2025-10-16 14:59 ` [PATCH nft 3/4] src: parser_bison: prevent multiple ip daddr/saddr definitions Florian Westphal
2025-10-16 23:41   ` Fernando Fernandez Mancera
2025-10-16 14:59 ` [PATCH nft 4/4] evaluate: reject tunnel section if another one is already present Florian Westphal
2025-10-16 23:44   ` Fernando Fernandez Mancera
2025-10-16 23:46 ` [PATCH nft 0/4] nft tunnel mode parser/eval fixes Fernando Fernandez Mancera
2025-10-17  9:39   ` Florian Westphal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).