From patchwork Tue Sep 28 12:16:48 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Westphal X-Patchwork-Id: 1533847 X-Patchwork-Delegate: pablo@netfilter.org Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=netfilter-devel-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by bilbo.ozlabs.org (Postfix) with ESMTP id 4HJdmC31lkz9svs for ; Tue, 28 Sep 2021 22:16:59 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240561AbhI1MSg (ORCPT ); Tue, 28 Sep 2021 08:18:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48582 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240631AbhI1MSf (ORCPT ); Tue, 28 Sep 2021 08:18:35 -0400 Received: from Chamillionaire.breakpoint.cc (Chamillionaire.breakpoint.cc [IPv6:2a0a:51c0:0:12e:520::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 37BE8C061604 for ; Tue, 28 Sep 2021 05:16:56 -0700 (PDT) Received: from fw by Chamillionaire.breakpoint.cc with local (Exim 4.92) (envelope-from ) id 1mVC2X-0006VN-QC; Tue, 28 Sep 2021 14:16:53 +0200 From: Florian Westphal To: Cc: Florian Westphal , Pablo Neira Ayuso Subject: [PATCH nft] payload: don't adjust offsets of autogenerated dependency expressions Date: Tue, 28 Sep 2021 14:16:48 +0200 Message-Id: <20210928121648.29581-1-fw@strlen.de> X-Mailer: git-send-email 2.32.0 MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Pablo says: user reports that this is broken: nft --debug=netlink add rule bridge filter forward vlan id 100 vlan id set 200 [..] [ payload load 2b @ link header + 14 => reg 1 ] [..] [ payload load 2b @ link header + 28 => reg 1 ] [ bitwise reg 1 = ( reg 1 & 0x000000f0 ) ^ 0x0000c800 ] [ payload write reg 1 => 2b @ link header + 14 csum_type 0 csum_off 0 csum_flags 0x0 ] offset says 28, it is assuming q-in-q, in this case it is mangling the existing header. The problem here is that 'vlan id set 200' needs a read-modify-write cycle because 'vlan id set' has to preserve bits located in the same byte area as the vlan id. The first 'payload load' at offset 14 is generated via 'vlan id 100', this part is ok. The second 'payload load' at offset 28 is the bogus one. Its added as a dependency, but then adjusted because nft evaluation considers this identical to 'vlan id 1 vlan id '2, where nft assumes q-in-q. To fix this, skip offset adjustments for raw expressions and mark the dependency-generated payload instruction as such. This is fine because raw payload operations assume that user specifies base/offset/length manually. Also add a test case for this. Reported-by: Pablo Neira Ayuso Signed-off-by: Florian Westphal --- src/evaluate.c | 1 + src/payload.c | 4 +++- tests/py/bridge/vlan.t | 3 +++ tests/py/bridge/vlan.t.json | 27 +++++++++++++++++++++++++++ tests/py/bridge/vlan.t.payload | 11 +++++++++++ tests/py/bridge/vlan.t.payload.netdev | 13 +++++++++++++ 6 files changed, 58 insertions(+), 1 deletion(-) diff --git a/src/evaluate.c b/src/evaluate.c index 8ebc75617b1c..b39f45981c42 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -2554,6 +2554,7 @@ static int stmt_evaluate_payload(struct eval_ctx *ctx, struct stmt *stmt) payload_byte_offset * BITS_PER_BYTE, payload_byte_size * BITS_PER_BYTE); + payload_bytes->payload.is_raw = 1; payload_bytes->payload.desc = payload->payload.desc; payload_bytes->byteorder = payload->byteorder; diff --git a/src/payload.c b/src/payload.c index 97b60713e800..c662900bdaac 100644 --- a/src/payload.c +++ b/src/payload.c @@ -115,7 +115,9 @@ static void payload_expr_pctx_update(struct proto_ctx *ctx, assert(desc->base <= PROTO_BASE_MAX); if (desc->base == base->base) { assert(base->length > 0); - ctx->protocol[base->base].offset += base->length; + + if (!left->payload.is_raw) + ctx->protocol[base->base].offset += base->length; } proto_ctx_update(ctx, desc->base, loc, desc); } diff --git a/tests/py/bridge/vlan.t b/tests/py/bridge/vlan.t index fd39d2227676..b506ee8df6bd 100644 --- a/tests/py/bridge/vlan.t +++ b/tests/py/bridge/vlan.t @@ -43,3 +43,6 @@ ether type 8021ad vlan id 1 vlan type 8021q vlan id 2 vlan type ip ip protocol 6 # illegal dependencies ether type ip vlan id 1;fail ether type ip vlan id 1 ip saddr 10.0.0.1;fail + +# mangling +vlan id 1 vlan id set 2;ok diff --git a/tests/py/bridge/vlan.t.json b/tests/py/bridge/vlan.t.json index dae70170398a..e7640f9a6a37 100644 --- a/tests/py/bridge/vlan.t.json +++ b/tests/py/bridge/vlan.t.json @@ -734,3 +734,30 @@ } } ] + +# vlan id 1 vlan id set 2 +[ + { + "match": { + "left": { + "payload": { + "field": "id", + "protocol": "vlan" + } + }, + "op": "==", + "right": 1 + } + }, + { + "mangle": { + "key": { + "payload": { + "field": "id", + "protocol": "vlan" + } + }, + "value": 2 + } + } +] diff --git a/tests/py/bridge/vlan.t.payload b/tests/py/bridge/vlan.t.payload index 49fd0ea7ab3b..6c8d595a1aad 100644 --- a/tests/py/bridge/vlan.t.payload +++ b/tests/py/bridge/vlan.t.payload @@ -265,3 +265,14 @@ bridge [ cmp eq reg 1 0x00000008 ] [ payload load 1b @ network header + 9 => reg 1 ] [ cmp eq reg 1 0x00000006 ] + +# vlan id 1 vlan id set 2 +bridge + [ payload load 2b @ link header + 12 => reg 1 ] + [ cmp eq reg 1 0x00000081 ] + [ payload load 2b @ link header + 14 => reg 1 ] + [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ] + [ cmp eq reg 1 0x00000100 ] + [ payload load 2b @ link header + 14 => reg 1 ] + [ bitwise reg 1 = ( reg 1 & 0x000000f0 ) ^ 0x00000200 ] + [ payload write reg 1 => 2b @ link header + 14 csum_type 0 csum_off 0 csum_flags 0x0 ] diff --git a/tests/py/bridge/vlan.t.payload.netdev b/tests/py/bridge/vlan.t.payload.netdev index 1a2c08ae7a94..d2c7d74a4e85 100644 --- a/tests/py/bridge/vlan.t.payload.netdev +++ b/tests/py/bridge/vlan.t.payload.netdev @@ -309,3 +309,16 @@ netdev [ cmp eq reg 1 0x00000008 ] [ payload load 1b @ network header + 9 => reg 1 ] [ cmp eq reg 1 0x00000006 ] + +# vlan id 1 vlan id set 2 +netdev + [ meta load iiftype => reg 1 ] + [ cmp eq reg 1 0x00000001 ] + [ payload load 2b @ link header + 12 => reg 1 ] + [ cmp eq reg 1 0x00000081 ] + [ payload load 2b @ link header + 14 => reg 1 ] + [ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ] + [ cmp eq reg 1 0x00000100 ] + [ payload load 2b @ link header + 14 => reg 1 ] + [ bitwise reg 1 = ( reg 1 & 0x000000f0 ) ^ 0x00000200 ] + [ payload write reg 1 => 2b @ link header + 14 csum_type 0 csum_off 0 csum_flags 0x0 ]