netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeremy Sowden <jeremy@azazel.net>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Netfilter Devel <netfilter-devel@vger.kernel.org>
Subject: [PATCH nft v3] evaluate: fix expr_set_context call for shift binops.
Date: Mon,  6 Jan 2020 22:35:10 +0000	[thread overview]
Message-ID: <20200106223510.496948-1-jeremy@azazel.net> (raw)
In-Reply-To: <20200106092842.tp2pxubgmfcptthq@salvia>

expr_evaluate_binop calls expr_set_context for shift expressions to set
the context data-type to `integer`.  This clobbers the byte-order of the
context, resulting in unexpected conversions to NBO.  For example:

  $ sudo nft flush ruleset
  $ sudo nft add table t
  $ sudo nft add chain t c '{ type filter hook output priority mangle; }'
  $ sudo nft add rule t c oif lo tcp dport ssh ct mark set '0x10 | 0xe'
  $ sudo nft add rule t c oif lo tcp dport ssh ct mark set '0xf << 1'
  $ sudo nft list table t
  table ip t {
          chain c {
                  type filter hook output priority mangle; policy accept;
                  oif "lo" tcp dport 22 ct mark set 0x0000001e
                  oif "lo" tcp dport 22 ct mark set 0x1e000000
          }
  }

Replace it with a call to __expr_set_context and set the byteorder to
that of the left operand since this is the value being shifted.

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

Since v2:

  * set the byte-order to that of the left operand, rather than hard-
    coding it as host-endian.

Since v1:

  * replace expr_set_context with __expr_set_context (and explicity set
    the byte-order) instead of removing it altogether in order to ensure
    that the right operand has integer type.

diff --git a/src/evaluate.c b/src/evaluate.c
index 817b23220bb9..34e4473e4c9a 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1145,7 +1145,8 @@ static int expr_evaluate_binop(struct eval_ctx *ctx, struct expr **expr)
 	left = op->left;
 
 	if (op->op == OP_LSHIFT || op->op == OP_RSHIFT)
-		expr_set_context(&ctx->ectx, &integer_type, ctx->ectx.len);
+		__expr_set_context(&ctx->ectx, &integer_type,
+				   left->byteorder, ctx->ectx.len, 0);
 	if (expr_evaluate(ctx, &op->right) < 0)
 		return -1;
 	right = op->right;
-- 
2.24.1


  parent reply	other threads:[~2020-01-06 22:35 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-20 19:02 [PATCH nftables] evaluate: remove expr_set_context call Jeremy Sowden
2019-12-24 23:12 ` Jeremy Sowden
2019-12-24 23:16   ` Jeremy Sowden
2019-12-24 23:14 ` [PATCH nft v2] evaluate: fix expr_set_context call for shift binops Jeremy Sowden
2020-01-06  9:28   ` Pablo Neira Ayuso
2020-01-06  9:31     ` Jeremy Sowden
2020-01-06 22:35     ` Jeremy Sowden [this message]
2020-01-08 22:35       ` [PATCH nft v3] " Pablo Neira Ayuso

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=20200106223510.496948-1-jeremy@azazel.net \
    --to=jeremy@azazel.net \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=pablo@netfilter.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).