src: move payload sub-byte matching to the evaluation step

Generating the bitwise logic to match sub-byte payload fields from the
linearize step has several problems:

1) When the bits are split between two bytes and the payload field is
   smaller than one byte, we need to extend the expression length on
   both sides (payload and constant) of the relational expression.

2) Explicit bitmask operations on sub-byte payload fields need to be
   merge to the implicit bitmask operation, otherwise we generate two
   bitwise instructions. This is not resolved by this patch, but we
   should have a look at some point to this.

With this approach, we can benefit from the binary operation transfer
for shifts to provide a generic way to adjust the constant side of the
expression.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

2 files changed, 97 insertions, 107 deletions

diff --git a/src/evaluate.c b/src/evaluate.c
index 72a0e435..ab732616 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -362,30 +362,108 @@ conflict_resolution_gen_dependency(struct eval_ctx *ctx, int protocol,
 	return 0;
 }
 
+static uint8_t expr_offset_shift(const struct expr *expr, unsigned int offset)
+{
+	unsigned int new_offset, len;
+	int shift;
+
+	new_offset = offset % BITS_PER_BYTE;
+	len = round_up(expr->len, BITS_PER_BYTE);
+	shift = len - (new_offset + expr->len);
+	assert(shift >= 0);
+
+	return shift;
+}
+
+static void expr_evaluate_bits(struct eval_ctx *ctx, struct expr **exprp)
+{
+	struct expr *expr = *exprp, *and, *mask, *lshift, *off;
+	unsigned masklen;
+	uint8_t shift;
+	mpz_t bitmask;
+
+	switch (expr->ops->type) {
+	case EXPR_PAYLOAD:
+		shift = expr_offset_shift(expr, expr->payload.offset);
+		break;
+	case EXPR_EXTHDR:
+		shift = expr_offset_shift(expr, expr->exthdr.tmpl->offset);
+		break;
+	default:
+		BUG("Unknown expression %s\n", expr->ops->name);
+	}
+
+	masklen = expr->len + shift;
+	assert(masklen <= NFT_REG_SIZE * BITS_PER_BYTE);
+
+	mpz_init2(bitmask, masklen);
+	mpz_bitmask(bitmask, expr->len);
+	mpz_lshift_ui(bitmask, shift);
+
+	mask = constant_expr_alloc(&expr->location, expr_basetype(expr),
+				   BYTEORDER_HOST_ENDIAN, masklen, NULL);
+	mpz_set(mask->value, bitmask);
+
+	and = binop_expr_alloc(&expr->location, OP_AND, expr, mask);
+	and->dtype	= expr->dtype;
+	and->byteorder	= expr->byteorder;
+	and->len	= masklen;
+
+	if (shift) {
+		off = constant_expr_alloc(&expr->location,
+					  expr_basetype(expr),
+					  BYTEORDER_BIG_ENDIAN,
+					  sizeof(shift), &shift);
+
+		lshift = binop_expr_alloc(&expr->location, OP_RSHIFT, and, off);
+		lshift->dtype		= expr->dtype;
+		lshift->byteorder	= expr->byteorder;
+		lshift->len		= masklen;
+
+		*exprp = lshift;
+	} else
+		*exprp = and;
+}
+
+static int __expr_evaluate_exthdr(struct eval_ctx *ctx, struct expr **exprp)
+{
+	struct expr *expr = *exprp;
+
+	if (expr_evaluate_primary(ctx, exprp) < 0)
+		return -1;
+
+	if (expr->exthdr.tmpl->offset % BITS_PER_BYTE != 0 ||
+	    expr->len % BITS_PER_BYTE != 0)
+		expr_evaluate_bits(ctx, exprp);
+
+	return 0;
+}
+
 /*
  * Exthdr expression: check whether dependencies are fulfilled, otherwise
  * generate the necessary relational expression and prepend it to the current
  * statement.
  */
-static int expr_evaluate_exthdr(struct eval_ctx *ctx, struct expr **expr)
+static int expr_evaluate_exthdr(struct eval_ctx *ctx, struct expr **exprp)
 {
 	const struct proto_desc *base;
+	struct expr *expr = *exprp;
 	struct stmt *nstmt;
 
 	base = ctx->pctx.protocol[PROTO_BASE_NETWORK_HDR].desc;
 	if (base == &proto_ip6)
-		return expr_evaluate_primary(ctx, expr);
+		return __expr_evaluate_exthdr(ctx, exprp);
 
 	if (base)
-		return expr_error(ctx->msgs, *expr,
+		return expr_error(ctx->msgs, expr,
 				  "cannot use exthdr with %s", base->name);
 
-	if (exthdr_gen_dependency(ctx, *expr, &nstmt) < 0)
+	if (exthdr_gen_dependency(ctx, expr, &nstmt) < 0)
 		return -1;
 
 	list_add(&nstmt->list, &ctx->rule->stmts);
 
-	return expr_evaluate_primary(ctx, expr);
+	return __expr_evaluate_exthdr(ctx, exprp);
 }
 
 /* dependency supersede.
@@ -509,12 +587,21 @@ static int __expr_evaluate_payload(struct eval_ctx *ctx, struct expr *expr)
 	return 0;
 }
 
-static int expr_evaluate_payload(struct eval_ctx *ctx, struct expr **expr)
+static int expr_evaluate_payload(struct eval_ctx *ctx, struct expr **exprp)
 {
-	if (__expr_evaluate_payload(ctx, *expr) < 0)
+	struct expr *expr = *exprp;
+
+	if (__expr_evaluate_payload(ctx, expr) < 0)
 		return -1;
 
-	return expr_evaluate_primary(ctx, expr);
+	if (expr_evaluate_primary(ctx, exprp) < 0)
+		return -1;
+
+	if (expr->payload.offset % BITS_PER_BYTE != 0 ||
+	    expr->len % BITS_PER_BYTE != 0)
+		expr_evaluate_bits(ctx, exprp);
+
+	return 0;
 }
 
 /*
@@ -1096,6 +1183,8 @@ static int binop_can_transfer(struct eval_ctx *ctx,
 						 "Comparison is always false");
 		return 1;
 	case OP_RSHIFT:
+		if (ctx->ectx.len < right->len + mpz_get_uint32(left->right->value))
+			ctx->ectx.len += mpz_get_uint32(left->right->value);
 		return 1;
 	case OP_XOR:
 		return 1;
diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c
index 32630436..e0c73c0f 100644
--- a/src/netlink_linearize.c
+++ b/src/netlink_linearize.c
@@ -104,64 +104,6 @@ static void netlink_gen_concat(struct netlink_linearize_ctx *ctx,
 	}
 }
 
-static unsigned int payload_shift_calc(const struct expr *expr,
-				       unsigned int offset)
-{
-	unsigned int len;
-	int shift;
-
-	offset %= BITS_PER_BYTE;
-	len = round_up(expr->len, BITS_PER_BYTE);
-	shift = len - (offset + expr->len);
-	assert(shift >= 0);
-
-	return shift;
-}
-
-static void netlink_gen_mask(struct netlink_linearize_ctx *ctx,
-			     const struct expr *expr,
-			     unsigned int shift,
-			     enum nft_registers dreg)
-{
-	struct nft_data_linearize nld, zero = {};
-	unsigned int len, masklen;
-	struct nftnl_expr *nle;
-	mpz_t mask;
-
-	masklen = expr->len + shift;
-	assert(masklen <= NFT_REG_SIZE * BITS_PER_BYTE);
-	mpz_init2(mask, masklen);
-	mpz_bitmask(mask, expr->len);
-	mpz_lshift_ui(mask, shift);
-
-	nle = alloc_nft_expr("bitwise");
-
-	len = div_round_up(expr->len, BITS_PER_BYTE);
-
-	nftnl_expr_set_u32(nle, NFT_EXPR_BITWISE_SREG, dreg);
-	nftnl_expr_set_u32(nle, NFT_EXPR_BITWISE_DREG, dreg);
-	nftnl_expr_set_u32(nle, NFT_EXPR_BITWISE_LEN, len);
-
-	netlink_gen_raw_data(mask, expr->byteorder, len, &nld);
-	nftnl_expr_set(nle, NFT_EXPR_BITWISE_MASK, nld.value, nld.len);
-	nftnl_expr_set(nle, NFT_EXPR_BITWISE_XOR, &zero.value, nld.len);
-
-	mpz_clear(mask);
-	nftnl_rule_add_expr(ctx->nlr, nle);
-}
-
-static void netlink_gen_payload_mask(struct netlink_linearize_ctx *ctx,
-				     const struct expr *expr,
-				     enum nft_registers dreg)
-{
-	unsigned int shift, offset;
-
-	offset = expr->payload.offset % BITS_PER_BYTE;
-	shift = payload_shift_calc(expr, offset);
-	if (shift || offset)
-		netlink_gen_mask(ctx, expr, shift, dreg);
-}
-
 static void netlink_gen_payload(struct netlink_linearize_ctx *ctx,
 				const struct expr *expr,
 				enum nft_registers dreg)
@@ -178,20 +120,6 @@ static void netlink_gen_payload(struct netlink_linearize_ctx *ctx,
 			   div_round_up(expr->len, BITS_PER_BYTE));
 
 	nftnl_rule_add_expr(ctx->nlr, nle);
-
-	netlink_gen_payload_mask(ctx, expr, dreg);
-}
-
-static void netlink_gen_exthdr_mask(struct netlink_linearize_ctx *ctx,
-				    const struct expr *expr,
-				    enum nft_registers dreg)
-{
-	unsigned int shift, offset;
-
-	offset = expr->exthdr.tmpl->offset % BITS_PER_BYTE;
-	shift = payload_shift_calc(expr, offset);
-	if (shift || offset)
-		netlink_gen_mask(ctx, expr, shift, dreg);
 }
 
 static void netlink_gen_exthdr(struct netlink_linearize_ctx *ctx,
@@ -209,8 +137,6 @@ static void netlink_gen_exthdr(struct netlink_linearize_ctx *ctx,
 	nftnl_expr_set_u32(nle, NFTNL_EXPR_EXTHDR_LEN,
 			   div_round_up(expr->len, BITS_PER_BYTE));
 	nftnl_rule_add_expr(ctx->nlr, nle);
-
-	netlink_gen_exthdr_mask(ctx, expr, dreg);
 }
 
 static void netlink_gen_meta(struct netlink_linearize_ctx *ctx,
@@ -319,28 +245,6 @@ static void netlink_gen_range(struct netlink_linearize_ctx *ctx,
 			      const struct expr *expr,
 			      enum nft_registers dreg);
 
-static void payload_shift_value(const struct expr *left, struct expr *right)
-{
-	unsigned int offset;
-
-	if (right->ops->type != EXPR_VALUE)
-		return;
-
-	switch (left->ops->type) {
-	case EXPR_PAYLOAD:
-		offset = left->payload.offset;
-		break;
-	case EXPR_EXTHDR:
-		offset = left->exthdr.tmpl->offset;
-		break;
-	default:
-		return;
-	}
-
-	mpz_lshift_ui(right->value,
-			payload_shift_calc(left, offset));
-}
-
 static struct expr *netlink_gen_prefix(struct netlink_linearize_ctx *ctx,
 				       const struct expr *expr,
 				       enum nft_registers sreg)
@@ -409,7 +313,6 @@ static void netlink_gen_cmp(struct netlink_linearize_ctx *ctx,
 	netlink_put_register(nle, NFTNL_EXPR_CMP_SREG, sreg);
 	nftnl_expr_set_u32(nle, NFTNL_EXPR_CMP_OP,
 			   netlink_gen_cmp_op(expr->op));
-	payload_shift_value(expr->left, right);
 	netlink_gen_data(right, &nld);
 	nftnl_expr_set(nle, NFTNL_EXPR_CMP_DATA, nld.value, len);
 	release_register(ctx, expr->left);
@@ -447,7 +350,6 @@ static void netlink_gen_range(struct netlink_linearize_ctx *ctx,
 		BUG("invalid range operation %u\n", expr->op);
 	}
 
-	payload_shift_value(expr->left, range->left);
 	netlink_gen_data(range->left, &nld);
 	nftnl_expr_set(nle, NFTNL_EXPR_CMP_DATA, nld.value, nld.len);
 	nftnl_rule_add_expr(ctx->nlr, nle);
@@ -468,7 +370,6 @@ static void netlink_gen_range(struct netlink_linearize_ctx *ctx,
 		BUG("invalid range operation %u\n", expr->op);
 	}
 
-	payload_shift_value(expr->left, range->right);
 	netlink_gen_data(range->right, &nld);
 	nftnl_expr_set(nle, NFTNL_EXPR_CMP_DATA, nld.value, nld.len);
 	nftnl_rule_add_expr(ctx->nlr, nle);