Blob Blame Raw
From 785823a1f607a7bcd32d4cb42655422c223fcad5 Mon Sep 17 00:00:00 2001
From: Phil Sutter <psutter@redhat.com>
Date: Mon, 7 Dec 2020 18:25:20 +0100
Subject: [PATCH] evaluate: Perform set evaluation on implicitly declared
 (anonymous) sets

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1877022
Upstream Status: nftables commit 7aa08d45031ec

commit 7aa08d45031ec7ce5dadb4979471d626367c09cd
Author: Stefano Brivio <sbrivio@redhat.com>
Date:   Wed May 27 22:51:21 2020 +0200

    evaluate: Perform set evaluation on implicitly declared (anonymous) sets

    If a set is implicitly declared, set_evaluate() is not called as a
    result of cmd_evaluate_add(), because we're adding in fact something
    else (e.g. a rule). Expression-wise, evaluation still happens as the
    implicit set expression is eventually found in the tree and handled
    by expr_evaluate_set(), but context-wise evaluation (set_evaluate())
    is skipped, and this might be relevant instead.

    This is visible in the reported case of an anonymous set including
    concatenated ranges:

      # nft add rule t c ip saddr . tcp dport { 192.0.2.1 . 20-30 } accept
      BUG: invalid range expression type concat
      nft: expression.c:1160: range_expr_value_low: Assertion `0' failed.
      Aborted

    because we reach do_add_set() without properly evaluated flags and
    set description, and eventually end up in expr_to_intervals(), which
    can't handle that expression.

    Explicitly call set_evaluate() as we add anonymous sets into the
    context, and instruct the same function to:
    - skip expression-wise set evaluation if the set is anonymous, as
      that happens later anyway as part of the general tree evaluation
    - skip the insertion in the set cache, as it makes no sense to have
      sets that shouldn't be referenced there

    For object maps, the allocation of the expression for set->data is
    already handled by set_evaluate(), so we can now drop that from
    stmt_evaluate_objref_map().

    v2:
     - skip insertion of set in cache (Pablo Neira Ayuso)
     - drop double allocation of expression (and leak of the first
       one) for object maps (Pablo Neira Ayuso)

    Reported-by: Pablo Neira Ayuso <pablo@netfilter.org>
    Reported-by: Phil Sutter <phil@nwl.cc>
    Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
    Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/evaluate.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 578dcae..fc45cef 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -75,6 +75,7 @@ static void key_fix_dtype_byteorder(struct expr *key)
 	datatype_set(key, set_datatype_alloc(dtype, key->byteorder));
 }
 
+static int set_evaluate(struct eval_ctx *ctx, struct set *set);
 static struct expr *implicit_set_declaration(struct eval_ctx *ctx,
 					     const char *name,
 					     struct expr *key,
@@ -105,6 +106,8 @@ static struct expr *implicit_set_declaration(struct eval_ctx *ctx,
 		list_add_tail(&cmd->list, &ctx->cmd->list);
 	}
 
+	set_evaluate(ctx, set);
+
 	return set_ref_expr_alloc(&expr->location, set);
 }
 
@@ -3171,12 +3174,6 @@ static int stmt_evaluate_objref_map(struct eval_ctx *ctx, struct stmt *stmt)
 
 		mappings = implicit_set_declaration(ctx, "__objmap%d",
 						    key, mappings);
-
-		mappings->set->data = constant_expr_alloc(&netlink_location,
-							  &string_type,
-							  BYTEORDER_HOST_ENDIAN,
-							  NFT_OBJ_MAXNAMELEN * BITS_PER_BYTE,
-							  NULL);
 		mappings->set->objtype  = stmt->objref.type;
 
 		map->mappings = mappings;
@@ -3381,6 +3378,13 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
 
 	}
 
+	/* Default timeout value implies timeout support */
+	if (set->timeout)
+		set->flags |= NFT_SET_TIMEOUT;
+
+	if (set_is_anonymous(set->flags))
+		return 0;
+
 	ctx->set = set;
 	if (set->init != NULL) {
 		expr_set_context(&ctx->ectx, set->key->dtype, set->key->len);
@@ -3392,10 +3396,6 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set)
 	if (set_lookup(table, set->handle.set.name) == NULL)
 		set_add_hash(set_get(set), table);
 
-	/* Default timeout value implies timeout support */
-	if (set->timeout)
-		set->flags |= NFT_SET_TIMEOUT;
-
 	return 0;
 }
 
-- 
1.8.3.1