On Sun, Jul 21, 2019 at 06:29:48PM +0200, Florian Westphal wrote:
> nft dumps core when a statement contains a prefix expression:
> iifname ens3 snat to 10.0.0.0/28
>
> yields:
> BUG: unknown expression type prefix
> nft: netlink_linearize.c:688: netlink_gen_expr: Assertion `0' failed.
>
> This assertion is correct -- we can't linearize a prefix because kernel
> doesn't know what that is.
>
> For LHS, they get converted to a binary 'and' such as
> '10.0.0.0 & 255.255.255.240'. For RHS, we can convert them into a range:
>
> iifname "ens3" snat to 10.0.0.0-10.0.0.15
>
> Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1187
> Signed-off-by: Florian Westphal <f...@strlen.de>
> ---
> src/evaluate.c | 61 +++++++++++++++++++++++++++++++++
> tests/py/ip6/dnat.t | 2 ++
> tests/py/ip6/dnat.t.json | 27 +++++++++++++++
> tests/py/ip6/dnat.t.payload.ip6 | 12 +++++++
> 4 files changed, 102 insertions(+)
>
> diff --git a/src/evaluate.c b/src/evaluate.c
> index 8c1c82abed4e..d55fe8ebb78e 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -1933,6 +1933,65 @@ static int stmt_evaluate_expr(struct eval_ctx *ctx,
> struct stmt *stmt)
> return expr_evaluate(ctx, &stmt->expr);
> }
>
> +static int stmt_prefix_conversion(struct eval_ctx *ctx, struct expr **expr,
> + enum byteorder byteorder)
> +{
> + struct expr *mask, *and, *or, *prefix, *base, *range;
> +
> + prefix = *expr;
> + base = prefix->prefix;
> +
> + if (base->etype != EXPR_VALUE)
> + return expr_error(ctx->msgs, prefix,
> + "you cannot specify a prefix here, "
> + "unknown type %s", base->dtype->name);
> +
> + if (!expr_is_constant(base))
> + return expr_error(ctx->msgs, prefix,
> + "Prefix expression is undefined for "
> + "non-constant expressions");
> +
> + if (expr_basetype(base)->type != TYPE_INTEGER)
> + return expr_error(ctx->msgs, prefix,
> + "Prefix expression expected integer value");
> +
> + mask = constant_expr_alloc(&prefix->location, expr_basetype(base),
> + BYTEORDER_HOST_ENDIAN, base->len, NULL);
> +
> + mpz_prefixmask(mask->value, base->len, prefix->prefix_len);
> + and = binop_expr_alloc(&prefix->location, OP_AND, expr_get(base), mask);
> +
> + if (expr_evaluate(ctx, &and) < 0) {
I think we don't need this expr_evaluate() call. The one at the bottom
of this function (for the range expression) should take care of this
already since expr_evaluate() makes recursive calls to left and right
hand side.
> + expr_free(and);
> + goto err_evaluation;
> + }
> +
> + mask = constant_expr_alloc(&prefix->location, expr_basetype(base),
> + BYTEORDER_HOST_ENDIAN, base->len, NULL);
> + mpz_bitmask(mask->value, prefix->len - prefix->prefix_len);
> + or = binop_expr_alloc(&prefix->location, OP_OR, expr_get(base), mask);
> +
> + if (expr_evaluate(ctx, &or) < 0) {
Same here.
> + expr_free(and);
> + expr_free(or);
> + goto err_evaluation;
> + }
> +
> + range = range_expr_alloc(&prefix->location, and, or);
> + if (expr_evaluate(ctx, &range) < 0) {
> + expr_free(range);
> + goto err_evaluation;
> + }
> +
> + expr_free(*expr);
> + *expr = range;
> + return 0;
> +
> +err_evaluation:
> + return expr_error(ctx->msgs, prefix,
> + "Could not expand prefix expression");
If this happens, then expr_error() will reported via recursive call
inside expr_evaluate(). I mean, expr_evaluate() itself should give us
an error already. So I would remove this, which is already actually
telling about an internal problem. The user cannot do much about this.
So either remove it or BUG() here I'd suggest.
Thanks!
