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 <kaber@trash.net>

diff --git a/src/netlink.c b/src/netlink.c
index 6e797dcfe16f9b790d3b167d531271e5312990c4..07af1cb80ec926ea8926bf82da7ac0bbc2e9dcc5 100644 (file)
--- 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 645bf63e32ffd66c173d5ea7e869aa8e34524e83..0e75c8a979ef010c1078c6c2271afc0a5c2e697d 100644 (file)
--- 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 e5fb536b53914a765cd5e73bb22b3e07b141eaca..9d59374c327289f9c6cda6ecbdadfaf6368bef7f 100644 (file)
--- 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;
        }