From patchwork Wed May 27 20:51:21 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefano Brivio X-Patchwork-Id: 1299221 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=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=izuNofmM; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 49XNKv59QBz9sSW for ; Thu, 28 May 2020 06:51:47 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728715AbgE0Uvr (ORCPT ); Wed, 27 May 2020 16:51:47 -0400 Received: from us-smtp-1.mimecast.com ([207.211.31.81]:51072 "EHLO us-smtp-delivery-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728571AbgE0Uvq (ORCPT ); Wed, 27 May 2020 16:51:46 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1590612705; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=oh4H839+SxmSLDEwccxLq90SaWwJvHB2N7qu0JEJSIA=; b=izuNofmM8rcbMgLUx9u0wH+F9y0r5aVmDtvuuQDuj9LFV2EmJbffWfkUhOiUNuQuk3ajuD r3ZIpoyR8CgK4FWgd5X2/lXHreAZC0bUn2Mr/79kh4OsxOw6soG/uC9fWtyVP7l1NwrEvB vT4J1TGP1Hyyam6SiiZon2LIW+x0mgo= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-299-TA0HlPYMNMew9a5pYJyfYQ-1; Wed, 27 May 2020 16:51:41 -0400 X-MC-Unique: TA0HlPYMNMew9a5pYJyfYQ-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B0B681005510; Wed, 27 May 2020 20:51:40 +0000 (UTC) Received: from epycfail.redhat.com (unknown [10.36.110.54]) by smtp.corp.redhat.com (Postfix) with ESMTP id 94EFF78B33; Wed, 27 May 2020 20:51:34 +0000 (UTC) From: Stefano Brivio To: Pablo Neira Ayuso Cc: Phil Sutter , netfilter-devel@vger.kernel.org Subject: [PATCH nft v2 1/2] evaluate: Perform set evaluation on implicitly declared (anonymous) sets Date: Wed, 27 May 2020 22:51:21 +0200 Message-Id: <03583cfdf6ad252f3c0472616a9e63178ee9d6ae.1590612113.git.sbrivio@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org If a set is implicitly declared, set_evaluate() is not called as a result of cmd_evaluate_add(), because we're adding in fact something else (e.g. a rule). Expression-wise, evaluation still happens as the implicit set expression is eventually found in the tree and handled by expr_evaluate_set(), but context-wise evaluation (set_evaluate()) is skipped, and this might be relevant instead. This is visible in the reported case of an anonymous set including concatenated ranges: # nft add rule t c ip saddr . tcp dport { 192.0.2.1 . 20-30 } accept BUG: invalid range expression type concat nft: expression.c:1160: range_expr_value_low: Assertion `0' failed. Aborted because we reach do_add_set() without properly evaluated flags and set description, and eventually end up in expr_to_intervals(), which can't handle that expression. Explicitly call set_evaluate() as we add anonymous sets into the context, and instruct the same function to: - skip expression-wise set evaluation if the set is anonymous, as that happens later anyway as part of the general tree evaluation - skip the insertion in the set cache, as it makes no sense to have sets that shouldn't be referenced there For object maps, the allocation of the expression for set->data is already handled by set_evaluate(), so we can now drop that from stmt_evaluate_objref_map(). v2: - skip insertion of set in cache (Pablo Neira Ayuso) - drop double allocation of expression (and leak of the first one) for object maps (Pablo Neira Ayuso) Reported-by: Pablo Neira Ayuso Reported-by: Phil Sutter Signed-off-by: Stefano Brivio --- src/evaluate.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/evaluate.c b/src/evaluate.c index 506f2c6a257e..9019458be53e 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -76,6 +76,7 @@ static void key_fix_dtype_byteorder(struct expr *key) datatype_set(key, set_datatype_alloc(dtype, key->byteorder)); } +static int set_evaluate(struct eval_ctx *ctx, struct set *set); static struct expr *implicit_set_declaration(struct eval_ctx *ctx, const char *name, struct expr *key, @@ -107,6 +108,8 @@ static struct expr *implicit_set_declaration(struct eval_ctx *ctx, list_add_tail(&cmd->list, &ctx->cmd->list); } + set_evaluate(ctx, set); + return set_ref_expr_alloc(&expr->location, set); } @@ -3316,12 +3319,6 @@ static int stmt_evaluate_objref_map(struct eval_ctx *ctx, struct stmt *stmt) mappings = implicit_set_declaration(ctx, "__objmap%d", key, mappings); - - mappings->set->data = constant_expr_alloc(&netlink_location, - &string_type, - BYTEORDER_HOST_ENDIAN, - NFT_OBJ_MAXNAMELEN * BITS_PER_BYTE, - NULL); mappings->set->objtype = stmt->objref.type; map->mappings = mappings; @@ -3546,6 +3543,13 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set) } + /* Default timeout value implies timeout support */ + if (set->timeout) + set->flags |= NFT_SET_TIMEOUT; + + if (set_is_anonymous(set->flags)) + return 0; + ctx->set = set; if (set->init != NULL) { __expr_set_context(&ctx->ectx, set->key->dtype, @@ -3558,10 +3562,6 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set) if (set_lookup(table, set->handle.set.name) == NULL) set_add_hash(set_get(set), table); - /* Default timeout value implies timeout support */ - if (set->timeout) - set->flags |= NFT_SET_TIMEOUT; - return 0; } From patchwork Wed May 27 20:51:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefano Brivio X-Patchwork-Id: 1299220 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=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=X53PQWEv; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 49XNKq4HVvz9sSF for ; Thu, 28 May 2020 06:51:43 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728709AbgE0Uvn (ORCPT ); Wed, 27 May 2020 16:51:43 -0400 Received: from us-smtp-2.mimecast.com ([207.211.31.81]:46112 "EHLO us-smtp-delivery-1.mimecast.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1728571AbgE0Uvm (ORCPT ); Wed, 27 May 2020 16:51:42 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1590612700; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=q8zH8SFS1b68Pe85NsoesUk0EGtyyf56k1MMHtVM0u0=; b=X53PQWEvrrYxjnEtrgklWBaAwW9IxHslItc/EDSKL/N6SxZxqreW/DYQGvsMqJhAEXLVz5 RTnWjvlccwvt6DOUOFqrzwGr8eiiCl6S/TOWwRMaIXS2veuoi7azuupg8AaqiKrs+SZWlo NBSVL6kgTau7tsldBaO+XT+gbV59XSc= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-141-cy3WZeUfO7m10x7CmL0qPg-1; Wed, 27 May 2020 16:51:38 -0400 X-MC-Unique: cy3WZeUfO7m10x7CmL0qPg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 730651005510; Wed, 27 May 2020 20:51:37 +0000 (UTC) Received: from epycfail.redhat.com (unknown [10.36.110.54]) by smtp.corp.redhat.com (Postfix) with ESMTP id 287C779C4D; Wed, 27 May 2020 20:51:35 +0000 (UTC) From: Stefano Brivio To: Pablo Neira Ayuso Cc: Phil Sutter , netfilter-devel@vger.kernel.org Subject: [PATCH nft v2 2/2] tests: py: Enable anonymous set rule with concatenated ranges in inet/sets.t Date: Wed, 27 May 2020 22:51:22 +0200 Message-Id: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org Commit 64b9aa3803dd ("tests/py: Add tests involving concatenated ranges") introduced a rule, commented out, adding an anonymous set including concatenated ranges. Now that they are properly handled, we can enable it. Note that this introduces a new warning. In the output below, '\' marks newlines I introduced to keep lines short: inet/sets.t: WARNING: line 24: \ 'add rule inet test-inet input ip daddr . tcp dport \ { 10.0.0.0/8 . 10-23, 192.168.1.1-192.168.3.8 . 80-443 } accept': \ 'ip daddr . tcp dport \ { 10.0.0.0/8 . 10-23, 192.168.1.1-192.168.3.8 . 80-443 } accept' \ mismatches 'meta nfproto ipv4 ip daddr . tcp dport \ { 10.0.0.0/8 . 10-23, 192.168.1.1-192.168.3.8 . 80-443} accept' which is similar to the existing warning, also introduced by commit 64b9aa3803dd: inet/sets.t: WARNING: line 23: \ 'add rule inet test-inet input \ ip saddr . ip daddr . tcp dport @set3 accept': \ 'ip saddr . ip daddr . tcp dport @set3 accept' mismatches \ 'meta nfproto ipv4 ip saddr . ip daddr . tcp dport @set3 accept' This is mentioned in the commit message for 64b9aa3803dd itself: * Payload dependency killing ignores the concatenated IP header expressions on LHS, so rule output is asymmetric. which means that for family inet, 'meta nfproto ipv4' is added to the output of the rule, on top of what was passed as input, but not for families bridge and netdev. For this reason, it's not possible in this case to specify a single expected output, differing from the input, and, also, 'meta nfproto ipv4' can only be passed as input for family inet as it's not relevant for the other families. As an alternative, we could split the rules from this test into tests for the corresponding families, as this test case itself is under the 'inet' directory, but I consider this beyond the scope of this patchset. v2: Enable rule in py/inet/sets.t instead of adding a new test in shell/sets (Phil Sutter) Signed-off-by: Stefano Brivio --- tests/py/inet/sets.t | 2 +- tests/py/inet/sets.t.payload.bridge | 14 ++++++++++++++ tests/py/inet/sets.t.payload.inet | 13 +++++++++++++ tests/py/inet/sets.t.payload.netdev | 13 +++++++++++++ 4 files changed, 41 insertions(+), 1 deletion(-) diff --git a/tests/py/inet/sets.t b/tests/py/inet/sets.t index e0b0ee867f9b..1c6f32355acf 100644 --- a/tests/py/inet/sets.t +++ b/tests/py/inet/sets.t @@ -21,4 +21,4 @@ ip6 daddr @set1 drop;fail ?set3 10.0.0.0/8 . 192.168.1.3-192.168.1.9 . 1024-65535;ok ip saddr . ip daddr . tcp dport @set3 accept;ok --ip daddr . tcp dport { 10.0.0.0/8 . 10-23, 192.168.1.1-192.168.3.8 . 80-443 } accept;ok +ip daddr . tcp dport { 10.0.0.0/8 . 10-23, 192.168.1.1-192.168.3.8 . 80-443 } accept;ok diff --git a/tests/py/inet/sets.t.payload.bridge b/tests/py/inet/sets.t.payload.bridge index 089d9dd7a28d..92f5417c0bee 100644 --- a/tests/py/inet/sets.t.payload.bridge +++ b/tests/py/inet/sets.t.payload.bridge @@ -26,3 +26,17 @@ bridge [ lookup reg 1 set set3 ] [ immediate reg 0 accept ] +# ip daddr . tcp dport { 10.0.0.0/8 . 10-23, 192.168.1.1-192.168.3.8 . 80-443 } accept +__set%d test-inet 87 +__set%d test-inet 0 + element 0000000a 00000a00 : 0 [end] element 0101a8c0 00005000 : 0 [end] +bridge + [ meta load protocol => reg 1 ] + [ cmp eq reg 1 0x00000008 ] + [ meta load l4proto => reg 1 ] + [ cmp eq reg 1 0x00000006 ] + [ payload load 4b @ network header + 16 => reg 1 ] + [ payload load 2b @ transport header + 2 => reg 9 ] + [ lookup reg 1 set __set%d ] + [ immediate reg 0 accept ] + diff --git a/tests/py/inet/sets.t.payload.inet b/tests/py/inet/sets.t.payload.inet index c5acd6103a03..bd6e1b0fe19d 100644 --- a/tests/py/inet/sets.t.payload.inet +++ b/tests/py/inet/sets.t.payload.inet @@ -26,3 +26,16 @@ inet [ lookup reg 1 set set3 ] [ immediate reg 0 accept ] +# ip daddr . tcp dport { 10.0.0.0/8 . 10-23, 192.168.1.1-192.168.3.8 . 80-443 } accept +__set%d test-inet 87 +__set%d test-inet 0 + element 0000000a 00000a00 : 0 [end] element 0101a8c0 00005000 : 0 [end] +inet + [ meta load nfproto => reg 1 ] + [ cmp eq reg 1 0x00000002 ] + [ meta load l4proto => reg 1 ] + [ cmp eq reg 1 0x00000006 ] + [ payload load 4b @ network header + 16 => reg 1 ] + [ payload load 2b @ transport header + 2 => reg 9 ] + [ lookup reg 1 set __set%d ] + [ immediate reg 0 accept ] diff --git a/tests/py/inet/sets.t.payload.netdev b/tests/py/inet/sets.t.payload.netdev index 82994eabf48b..f3032d8ef4ab 100644 --- a/tests/py/inet/sets.t.payload.netdev +++ b/tests/py/inet/sets.t.payload.netdev @@ -26,3 +26,16 @@ inet [ lookup reg 1 set set3 ] [ immediate reg 0 accept ] +# ip daddr . tcp dport { 10.0.0.0/8 . 10-23, 192.168.1.1-192.168.3.8 . 80-443 } accept +__set%d test-netdev 87 +__set%d test-netdev 0 + element 0000000a 00000a00 : 0 [end] element 0101a8c0 00005000 : 0 [end] +netdev + [ meta load protocol => reg 1 ] + [ cmp eq reg 1 0x00000008 ] + [ meta load l4proto => reg 1 ] + [ cmp eq reg 1 0x00000006 ] + [ payload load 4b @ network header + 16 => reg 1 ] + [ payload load 2b @ transport header + 2 => reg 9 ] + [ lookup reg 1 set __set%d ] + [ immediate reg 0 accept ]