netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeremy Sowden <jeremy@azazel.net>
To: Netfilter Devel <netfilter-devel@vger.kernel.org>
Cc: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
Subject: [nft PATCH v4 21/32] evaluate: don't clobber binop lengths
Date: Mon,  4 Apr 2022 13:13:59 +0100	[thread overview]
Message-ID: <20220404121410.188509-22-jeremy@azazel.net> (raw)
In-Reply-To: <20220404121410.188509-1-jeremy@azazel.net>

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 --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);
-- 
2.35.1


  parent reply	other threads:[~2022-04-04 12:28 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-04 12:13 [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 01/32] examples: add .gitignore file Jeremy Sowden
2022-04-05 11:26   ` Florian Westphal
2022-04-04 12:13 ` [nft PATCH v4 02/32] include: add missing `#include` Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 03/32] src: move `byteorder_names` array Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 04/32] datatype: support `NULL` symbol-tables when printing constants Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 05/32] ct: support `NULL` symbol-tables when looking up labels Jeremy Sowden
2022-04-05 11:15   ` Florian Westphal
2022-04-05 15:29     ` Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 06/32] include: update nf_tables.h Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 07/32] include: add new bitwise bit-length attribute to nf_tables.h Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 08/32] netlink: send bit-length of bitwise binops to kernel Jeremy Sowden
2022-05-23 17:03   ` Pablo Neira Ayuso
2022-11-01 18:46     ` Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 09/32] netlink_delinearize: add postprocessing for payload binops Jeremy Sowden
2022-05-23 17:19   ` Pablo Neira Ayuso
2022-11-01 18:46     ` Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 10/32] netlink_delinearize: correct type and byte-order of shifts Jeremy Sowden
2022-05-23 17:19   ` Pablo Neira Ayuso
2022-11-01 18:47     ` Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 11/32] netlink_delinearize: correct length of right bitwise operand Jeremy Sowden
2022-05-23 17:22   ` Pablo Neira Ayuso
2022-11-01 18:47     ` Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 12/32] payload: set byte-order when completing expression Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 13/32] evaluate: support shifts larger than the width of the left operand Jeremy Sowden
2022-05-23 17:42   ` Pablo Neira Ayuso
2022-11-01 18:47     ` Jeremy Sowden
2023-02-07 12:05     ` Pablo Neira Ayuso
2023-03-04 12:00       ` Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 14/32] evaluate: relax type-checking for integer arguments in mark statements Jeremy Sowden
2022-05-23 17:33   ` Pablo Neira Ayuso
2022-04-04 12:13 ` [nft PATCH v4 15/32] tests: shell: rename some test-cases Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 16/32] tests: shell: add test-cases for ct and packet mark payload expressions Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 17/32] tests: py: " Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 18/32] include: add new bitwise boolean attributes to nf_tables.h Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 19/32] evaluate: don't eval unary arguments Jeremy Sowden
2022-04-04 12:13 ` [nft PATCH v4 20/32] evaluate: prevent nested byte-order conversions Jeremy Sowden
2022-04-04 12:13 ` Jeremy Sowden [this message]
2022-04-04 12:14 ` [nft PATCH v4 22/32] evaluate: insert byte-order conversions for expressions between 9 and 15 bits Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 23/32] evaluate: set eval context to leftmost bitwise operand Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 24/32] netlink_delinearize: fix typo Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 25/32] netlink_delinearize: refactor stmt_payload_binop_postprocess Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 26/32] netlink_delinearize: add support for processing variable payload statement arguments Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 27/32] netlink: rename bitwise operation functions Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 28/32] netlink: support (de)linearization of new bitwise boolean operations Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 29/32] parser_json: allow RHS ct, meta and payload expressions Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 30/32] evaluate: allow binop expressions with variable right-hand operands Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 31/32] tests: shell: add tests for binops with variable RHS operands Jeremy Sowden
2022-04-04 12:14 ` [nft PATCH v4 32/32] tests: py: " Jeremy Sowden
2022-04-09  8:30 ` [nft PATCH v4 00/32] Extend values assignable to packet marks and payload fields Kevin 'ldir' Darbyshire-Bryant

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220404121410.188509-22-jeremy@azazel.net \
    --to=jeremy@azazel.net \
    --cc=ldir@darbyshire-bryant.me.uk \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).