diff options
author | Pablo Neira Ayuso <pablo@netfilter.org> | 2023-12-06 18:48:29 +0100 |
---|---|---|
committer | Florian Westphal <fw@strlen.de> | 2023-12-08 19:33:28 +0100 |
commit | 8d3de823b622136e1d05a6fed11ff2dc0e804f8a (patch) | |
tree | dd000055fb4d59f77aebc15c9f640feedbc68102 /src | |
parent | 94fd162ea4d25fe6b0b4d58dcb7ff66dc55f3247 (diff) |
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>
Diffstat (limited to 'src')
-rw-r--r-- | src/evaluate.c | 26 | ||||
-rw-r--r-- | src/payload.c | 29 |
2 files changed, 25 insertions, 30 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: diff --git a/src/payload.c b/src/payload.c index 140ca50a..5de3d320 100644 --- a/src/payload.c +++ b/src/payload.c @@ -407,7 +407,6 @@ static int payload_add_dependency(struct eval_ctx *ctx, const struct proto_hdr_template *tmpl; struct expr *dep, *left, *right; struct proto_ctx *pctx; - unsigned int stmt_len; struct stmt *stmt; int protocol; @@ -429,15 +428,9 @@ static int payload_add_dependency(struct eval_ctx *ctx, dep = relational_expr_alloc(&expr->location, OP_EQ, left, right); - stmt_len = ctx->stmt_len; - ctx->stmt_len = 0; - stmt = expr_stmt_alloc(&dep->location, dep); - if (stmt_evaluate(ctx, stmt) < 0) { - return expr_error(ctx->msgs, expr, - "dependency statement is invalid"); - } - ctx->stmt_len = stmt_len; + if (stmt_dependency_evaluate(ctx, stmt) < 0) + return -1; if (ctx->inner_desc) { if (tmpl->meta_key) @@ -547,7 +540,6 @@ int payload_gen_dependency(struct eval_ctx *ctx, const struct expr *expr, const struct hook_proto_desc *h; const struct proto_desc *desc; struct proto_ctx *pctx; - unsigned int stmt_len; struct stmt *stmt; uint16_t type; @@ -564,17 +556,11 @@ int payload_gen_dependency(struct eval_ctx *ctx, const struct expr *expr, "protocol specification is invalid " "for this family"); - stmt_len = ctx->stmt_len; - ctx->stmt_len = 0; - stmt = meta_stmt_meta_iiftype(&expr->location, type); - if (stmt_evaluate(ctx, stmt) < 0) { - return expr_error(ctx->msgs, expr, - "dependency statement is invalid"); - } - *res = stmt; + if (stmt_dependency_evaluate(ctx, stmt) < 0) + return -1; - ctx->stmt_len = stmt_len; + *res = stmt; return 0; } @@ -1442,9 +1428,8 @@ int payload_gen_icmp_dependency(struct eval_ctx *ctx, const struct expr *expr, pctx->th_dep.icmp.type = type; - if (stmt_evaluate(ctx, stmt) < 0) - return expr_error(ctx->msgs, expr, - "icmp dependency statement is invalid"); + if (stmt_dependency_evaluate(ctx, stmt) < 0) + return -1; done: *res = stmt; return 0; |