evaluate: reset statement length context before evaluating statement

This patch consolidates ctx->stmt_len reset in stmt_evaluate() to avoid
this problem. Note that stmt_evaluate_meta() and stmt_evaluate_ct()
already reset it after the statement evaluation.

Moreover, statement dependency can be generated while evaluating a meta
and ct statement. Payload statement dependency already manually stashes
this before calling stmt_evaluate(). Add a new stmt_dependency_evaluate()
function to stash statement length context when evaluating a new statement
dependency and use it for all of the existing statement dependencies.

Florian also says:

'meta mark set vlan id map { 1 : 0x00000001, 4095 : 0x00004095 }' will
crash. Reason is that the l2 dependency generated here is errounously
expanded to a 32bit-one, so the evaluation path won't recognize this
as a L2 dependency.  Therefore, pctx->stacked_ll_count is 0 and
__expr_evaluate_payload() crashes with a null deref when
dereferencing pctx->stacked_ll[0].

nft-test.py gains a fugly hack to tolerate '!map typeof vlan id : meta mark'.
For more generic support we should find something more acceptable, e.g.

!map typeof( everything here is a key or data ) timeout ...

tests/py update and assert(pctx->stacked_ll_count) by Florian Westphal.

Fixes: edecd58755a8 ("evaluate: support shifts larger than the width of the left operand")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Florian Westphal <fw@strlen.de>

1 files changed, 18 insertions, 8 deletions

diff --git a/src/evaluate.c b/src/evaluate.c
index c32857c7..a62a2346 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -454,6 +454,18 @@ static int expr_evaluate_primary(struct eval_ctx *ctx, struct expr **expr)
 	return 0;
 }
 
+int stmt_dependency_evaluate(struct eval_ctx *ctx, struct stmt *stmt)
+{
+	uint32_t stmt_len = ctx->stmt_len;
+
+	if (stmt_evaluate(ctx, stmt) < 0)
+		return stmt_error(ctx, stmt, "dependency statement is invalid");
+
+	ctx->stmt_len = stmt_len;
+
+	return 0;
+}
+
 static int
 conflict_resolution_gen_dependency(struct eval_ctx *ctx, int protocol,
 				   const struct expr *expr,
@@ -479,7 +491,7 @@ conflict_resolution_gen_dependency(struct eval_ctx *ctx, int protocol,
 
 	dep = relational_expr_alloc(&expr->location, OP_EQ, left, right);
 	stmt = expr_stmt_alloc(&dep->location, dep);
-	if (stmt_evaluate(ctx, stmt) < 0)
+	if (stmt_dependency_evaluate(ctx, stmt) < 0)
 		return expr_error(ctx->msgs, expr,
 					  "dependency statement is invalid");
 
@@ -705,9 +717,8 @@ static int meta_iiftype_gen_dependency(struct eval_ctx *ctx,
 				  "for this family");
 
 	nstmt = meta_stmt_meta_iiftype(&payload->location, type);
-	if (stmt_evaluate(ctx, nstmt) < 0)
-		return expr_error(ctx->msgs, payload,
-				  "dependency statement is invalid");
+	if (stmt_dependency_evaluate(ctx, nstmt) < 0)
+		return -1;
 
 	if (ctx->inner_desc)
 		nstmt->expr->left->meta.inner_desc = ctx->inner_desc;
@@ -818,6 +829,7 @@ static int __expr_evaluate_payload(struct eval_ctx *ctx, struct expr *expr)
 						  desc->name,
 						  payload->payload.desc->name);
 
+			assert(pctx->stacked_ll_count);
 			payload->payload.offset += pctx->stacked_ll[0]->length;
 			rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
 			return 1;
@@ -3171,8 +3183,6 @@ static int stmt_evaluate_meta(struct eval_ctx *ctx, struct stmt *stmt)
 				stmt->meta.tmpl->len,
 				stmt->meta.tmpl->byteorder,
 				&stmt->meta.expr);
-	ctx->stmt_len = 0;
-
 	if (ret < 0)
 		return ret;
 
@@ -3200,8 +3210,6 @@ static int stmt_evaluate_ct(struct eval_ctx *ctx, struct stmt *stmt)
 				stmt->ct.tmpl->len,
 				stmt->ct.tmpl->byteorder,
 				&stmt->ct.expr);
-	ctx->stmt_len = 0;
-
 	if (ret < 0)
 		return -1;
 
@@ -4497,6 +4505,8 @@ int stmt_evaluate(struct eval_ctx *ctx, struct stmt *stmt)
 		erec_destroy(erec);
 	}
 
+	ctx->stmt_len = 0;
+
 	switch (stmt->ops->type) {
 	case STMT_CONNLIMIT:
 	case STMT_COUNTER: