diff mbox series

[nft,v4,21/32] evaluate: don't clobber binop lengths

Message ID 20220404121410.188509-22-jeremy@azazel.net
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series Extend values assignable to packet marks and payload fields | expand

Commit Message

Jeremy Sowden April 4, 2022, 12:13 p.m. UTC 
Binops with variable RHS operands will make it possible to do thing like
this:

  nft add rule t c ip dscp set ip dscp and 0xc

However, the netlink dump reveals a problem:

  [ payload load 2b @ network header + 0 => reg 1 ]
  [ bitwise reg 1 = ( reg 1 & 0x000003ff ) ^ 0x00000000 ]
  [ payload load 1b @ network header + 1 => reg 2 ]
  [ bitwise reg 2 = ( reg 2 & 0x0000003c ) ^ 0x00000000 ]
  [ bitwise reg 2 = ( reg 2 >> 0x00000002 ) ]
  [ bitwise reg 2 = ( reg 2 & 0x0000000c ) ^ 0x00000000 ]
  [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
  [ bitwise reg 1 = ( reg 1 ^ reg 2 ) ]
  [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]

The mask at line 4 should be 0xfc, not 0x3c.

Evaluation of the payload expression munges it from `ip dscp` to
`(ip dscp & 0xfc) >> 2`, because although `ip dscp` is only 6 bits long,
those 6 bits are the top bits in a byte, and to make the arithmetic
simpler when we perform comparisons and assignments, we mask and shift
the field.  When the AND expression is allocated, its length is
correctly set to 8.  However, when a binop is evaluated, it is assumed
that the length has not been set and it always set to the length of the
left operand, incorrectly to 6 in this case.  When the bitwise netlink
expression is generated, the length of the AND is used to generate the
mask, 0x3f, used in combining the binop's.  The upshot of this is that
the original mask gets mangled to 0x3c.

We can fix this by changing the evaluation of binops only to set the
op's length if it is not already set.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/evaluate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
diff mbox series

Patch

diff --git a/src/evaluate.c b/src/evaluate.c
index 3f697eb1dd43..e19f6300fe2c 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1121,7 +1121,7 @@  static int expr_evaluate_shift(struct eval_ctx *ctx, struct expr **expr)
 {
 	struct expr *op = *expr, *left = op->left, *right = op->right;
 	unsigned int shift = mpz_get_uint32(right->value);
-	unsigned int op_len = left->len;
+	unsigned int op_len = op->len ? : left->len;
 
 	if (shift >= op_len) {
 		if (shift >= ctx->ectx.len)
@@ -1158,7 +1158,7 @@  static int expr_evaluate_bitwise(struct eval_ctx *ctx, struct expr **expr)
 
 	op->dtype     = left->dtype;
 	op->byteorder = left->byteorder;
-	op->len	      = left->len;
+	op->len	      = op->len ? : left->len;
 
 	if (expr_is_constant(left))
 		return constant_binop_simplify(ctx, expr);