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>

diff --git a/src/evaluate.c b/src/evaluate.c
index 985ae4fe4aae438c9fe8cd33dbf10334e407ee11..4156d896fb5380229f19b1fca03e14bbee8ef811 100644 (file)
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -76,6 +76,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,
@@ -107,6 +108,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);
 }
 
@@ -3316,12 +3319,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;
@@ -3546,6 +3543,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,
@@ -3558,10 +3562,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;
 }