From 0545e0c13b3b7dff4dd53c8a68d8d1066c2829c0 Mon Sep 17 00:00:00 2001 From: Patrick McHardy Date: Mon, 17 Feb 2014 14:06:44 +0000 Subject: netlink: fix prefix expression handling The prefix expression handling is full of bugs: - netlink_gen_data() is used to construct the prefix mask from the full prefix expression. This is both conceptually wrong, the prefix expression is *not* data, and buggy, it only assumes network masks and thus only handles big endian types. - Prefix expression reconstruction doesn't check whether the mask is a valid prefix and reconstructs crap otherwise. It doesn't reconstruct prefixes for anything but network addresses. On top of that its needlessly complicated, using the mpz values directly its a simple matter of finding the sequence of 1's that extend up to the full width. - Unnecessary cloning of expressions where a simple refcount increase would suffice. Rewrite that code properly. Signed-off-by: Patrick McHardy --- src/netlink.c | 27 ------------------------ src/netlink_delinearize.c | 52 +++++++++++++++++++++-------------------------- src/netlink_linearize.c | 11 ++++++++-- 3 files changed, 32 insertions(+), 58 deletions(-) diff --git a/src/netlink.c b/src/netlink.c index 6e797dcf..07af1cb8 100644 --- a/src/netlink.c +++ b/src/netlink.c @@ -252,31 +252,6 @@ static void netlink_gen_verdict(const struct expr *expr, } } -static void netlink_gen_prefix(const struct expr *expr, - struct nft_data_linearize *data) -{ - uint32_t idx; - int32_t i, cidr; - uint32_t mask; - - assert(expr->ops->type == EXPR_PREFIX); - - data->len = div_round_up(expr->prefix->len, BITS_PER_BYTE); - cidr = expr->prefix_len; - - for (i = 0; (uint32_t)i / BITS_PER_BYTE < data->len; i += 32) { - if (cidr - i >= 32) - mask = 0xffffffff; - else if (cidr - i > 0) - mask = (1 << (cidr - i)) - 1; - else - mask = 0; - - idx = i / 32; - data->value[idx] = mask; - } -} - void netlink_gen_data(const struct expr *expr, struct nft_data_linearize *data) { switch (expr->ops->type) { @@ -286,8 +261,6 @@ void netlink_gen_data(const struct expr *expr, struct nft_data_linearize *data) return netlink_gen_concat_data(expr, data); case EXPR_VERDICT: return netlink_gen_verdict(expr, data); - case EXPR_PREFIX: - return netlink_gen_prefix(expr, data); default: BUG("invalid data expression type %s\n", expr->ops->name); } diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c index 645bf63e..0e75c8a9 100644 --- a/src/netlink_delinearize.c +++ b/src/netlink_delinearize.c @@ -663,33 +663,27 @@ static void meta_match_postprocess(struct rule_pp_ctx *ctx, } } -static int expr_value2cidr(struct expr *expr) +/* Convert a bitmask to a prefix length */ +static unsigned int expr_mask_to_prefix(struct expr *expr) { - int i, j, k = 0; - uint32_t data[4], bits; - uint32_t len = div_round_up(expr->len, BITS_PER_BYTE) / sizeof(uint32_t); + unsigned long n; - assert(expr->ops->type == EXPR_VALUE); - - mpz_export_data(data, expr->value, expr->byteorder, len); - - for (i = len - 1; i >= 0; i--) { - j = 32; - bits = UINT32_MAX >> 1; - - if (data[i] == UINT32_MAX) - goto next; + n = mpz_scan1(expr->value, 0); + return mpz_scan0(expr->value, n + 1) - n; +} - while (--j >= 0) { - if (data[i] == bits) - break; +/* Return true if a bitmask can be expressed as a prefix length */ +static bool expr_mask_is_prefix(const struct expr *expr) +{ + unsigned long n1, n2; - bits >>=1; - } -next: - k += j; - } - return k; + n1 = mpz_scan1(expr->value, 0); + if (n1 == ULONG_MAX) + return false; + n2 = mpz_scan0(expr->value, n1 + 1); + if (n2 < expr->len || n2 == ULONG_MAX) + return false; + return true; } /* Convert a series of inclusive OR expressions into a list */ @@ -729,13 +723,13 @@ static void relational_binop_postprocess(struct expr *expr) expr->op = OP_FLAGCMP; expr_free(binop); - } else if ((binop->left->dtype->type == TYPE_IPADDR || - binop->left->dtype->type == TYPE_IP6ADDR) && - binop->op == OP_AND) { - expr->left = expr_clone(binop->left); + } else if (binop->left->dtype->flags & DTYPE_F_PREFIX && + binop->op == OP_AND && + expr_mask_is_prefix(binop->right)) { + expr->left = expr_get(binop->left); expr->right = prefix_expr_alloc(&expr->location, - expr_clone(value), - expr_value2cidr(binop->right)); + expr_get(value), + expr_mask_to_prefix(binop->right)); expr_free(value); expr_free(binop); } diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c index e5fb536b..9d59374c 100644 --- a/src/netlink_linearize.c +++ b/src/netlink_linearize.c @@ -193,9 +193,14 @@ static void netlink_gen_cmp(struct netlink_linearize_ctx *ctx, netlink_gen_expr(ctx, expr->left, sreg); if (expr->right->ops->type == EXPR_PREFIX) { - right = expr->right->prefix; + mpz_t mask; + + mpz_init(mask); + mpz_prefixmask(mask, expr->right->len, expr->right->prefix_len); + netlink_gen_raw_data(mask, expr->right->byteorder, + expr->right->len / BITS_PER_BYTE, &nld); + mpz_clear(mask); - netlink_gen_data(expr->right, &nld); zero.len = nld.len; nle = alloc_nft_expr("bitwise"); @@ -205,6 +210,8 @@ static void netlink_gen_cmp(struct netlink_linearize_ctx *ctx, nft_rule_expr_set(nle, NFT_EXPR_BITWISE_MASK, &nld.value, nld.len); nft_rule_expr_set(nle, NFT_EXPR_BITWISE_XOR, &zero.value, zero.len); nft_rule_add_expr(ctx->nlr, nle); + + right = expr->right->prefix; } else { right = expr->right; } -- cgit v1.2.3