All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: <netfilter-devel@vger.kernel.org>
Cc: Florian Westphal <fw@strlen.de>
Subject: [PATCH v2 nft 2/3] src: do not merge a set with a erroneous one
Date: Wed, 10 Jan 2024 09:26:53 +0100	[thread overview]
Message-ID: <20240110082657.1967-3-fw@strlen.de> (raw)
In-Reply-To: <20240110082657.1967-1-fw@strlen.de>

The included sample causes a crash because we attempt to
range-merge a prefix expression with a symbolic expression.

This happens because the first set is evaluated, the symbol
expr evaluation fails, because the symbolic name cannot be resolved.

nft queues error message
("Could not resolve service: Servname not supported for ai_socktype")
*BUT CONTINUES PROCESSING* of the remaining ruleset.

It then encounters the same set definition again and merges the
new content with the preceeding one.

But the first set structure is dodgy, it still contains the
unresolved symbolic expression.

That then makes nft crash (assert) in the set internals.

There are various different incarnations of this issue, but the low
level set processing code does not allow for any partially transformed
expressions to still remain.

Before:
nft --check -f tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop
BUG: invalid range expression type binop
nft: src/expression.c:1479: range_expr_value_low: Assertion `0' failed.

After:
nft --check -f tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop
invalid_range_expr_type_binop:4:18-25: Error: Could not resolve hostname: Name or service not known
elements = { 1&.141.0.1 - 192.168.0.2}
             ^^^^^^^^

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/rule.h                                       |  2 ++
 src/evaluate.c                                       |  4 +++-
 src/intervals.c                                      |  3 +++
 .../bogons/nft-f/invalid_range_expr_type_binop       | 12 ++++++++++++
 4 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop

diff --git a/include/rule.h b/include/rule.h
index 6835fe069165..02fe2e1665e3 100644
--- a/include/rule.h
+++ b/include/rule.h
@@ -329,6 +329,7 @@ void rule_stmt_insert_at(struct rule *rule, struct stmt *nstmt,
  * @policy:	set mechanism policy
  * @automerge:	merge adjacents and overlapping elements, if possible
  * @comment:	comment
+ * @errors:	expr evaluation errors seen
  * @desc.size:		count of set elements
  * @desc.field_len:	length of single concatenated fields, bytes
  * @desc.field_count:	count of concatenated fields
@@ -353,6 +354,7 @@ struct set {
 	bool			root;
 	bool			automerge;
 	bool			key_typeof_valid;
+	bool			errors;
 	const char		*comment;
 	struct {
 		uint32_t	size;
diff --git a/src/evaluate.c b/src/evaluate.c
index eac9267a0107..4e9a95ad4c9d 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -4800,8 +4800,10 @@ static int elems_evaluate(struct eval_ctx *ctx, struct set *set)
 
 		__expr_set_context(&ctx->ectx, set->key->dtype,
 				   set->key->byteorder, set->key->len, 0);
-		if (expr_evaluate(ctx, &set->init) < 0)
+		if (expr_evaluate(ctx, &set->init) < 0) {
+			set->errors = true;
 			return -1;
+		}
 		if (set->init->etype != EXPR_SET)
 			return expr_error(ctx->msgs, set->init, "Set %s: Unexpected initial type %s, missing { }?",
 					  set->handle.set.name, expr_name(set->init));
diff --git a/src/intervals.c b/src/intervals.c
index ea39dbb80665..85b44d105402 100644
--- a/src/intervals.c
+++ b/src/intervals.c
@@ -142,6 +142,9 @@ static int set_sort_splice(struct list_head *msgs,
 	if (!existing_set)
 		return 0;
 
+	if (existing_set->errors)
+		return -1;
+
 	if (existing_set->init) {
 		err = set_to_range(msgs, existing_set->init);
 		if (err)
diff --git a/tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop b/tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop
new file mode 100644
index 000000000000..514d6ffe1319
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-f/invalid_range_expr_type_binop
@@ -0,0 +1,12 @@
+table ip x {
+	map z {
+		type ipv4_addr : ipv4_addr
+		elements = { 1&.141.0.1 - 192.168.0.2}
+	}
+
+	map z {
+		type ipv4_addr : ipv4_addr
+		flags interval
+		elements = { 10.141.0.0, * : 192.168.0.4 }
+	}
+}
-- 
2.41.0


  parent reply	other threads:[~2024-01-10  8:27 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-10  8:26 [PATCH v2 nft 0/3] set related parser fixes Florian Westphal
2024-01-10  8:26 ` [PATCH v2 nft 1/3] intervals: allow low-level interval code to return errors Florian Westphal
2024-01-10  8:26 ` Florian Westphal [this message]
2024-01-10  8:26 ` [PATCH v2 nft 3/3] evaluate: don't assert if set->data is NULL Florian Westphal
2024-01-10  8:44   ` Florian Westphal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240110082657.1967-3-fw@strlen.de \
    --to=fw@strlen.de \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.