From patchwork Tue Apr 5 12:43:00 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1613403 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=imp4U4vb; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4KXnPP2qqyz9sFk for ; Tue, 5 Apr 2022 22:43:21 +1000 (AEST) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id F194682C99; Tue, 5 Apr 2022 12:43:19 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id EMjRiFdCNq1s; Tue, 5 Apr 2022 12:43:19 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id 11AD0828A5; Tue, 5 Apr 2022 12:43:18 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id CF2D9C0012; Tue, 5 Apr 2022 12:43:17 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp3.osuosl.org (smtp3.osuosl.org [IPv6:2605:bc80:3010::136]) by lists.linuxfoundation.org (Postfix) with ESMTP id 9C037C0012 for ; Tue, 5 Apr 2022 12:43:16 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id 2019B61039 for ; Tue, 5 Apr 2022 12:43:09 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp3.osuosl.org (amavisd-new); dkim=pass (1024-bit key) header.d=redhat.com Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id HYadQ6BVIwho for ; Tue, 5 Apr 2022 12:43:08 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by smtp3.osuosl.org (Postfix) with ESMTPS id 4D3D161032 for ; Tue, 5 Apr 2022 12:43:08 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1649162587; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=v/LoVJ3GXBcVyD1V6iC3k84fa8ZZizix0ldSSCmf2Uo=; b=imp4U4vb3kK0QgNL749MZWnlAR0tBKJd9X8DNgeqpFgghiWr811cOQMDrkCXNqeXIAZJ0l Nt5sPnd3SrQKR5S7Y2CMHZsB4EEuBQntFs7y+bvvOFkiGq7coGNWIIijjvdajZIZqeg1Ui Qr0MSpY7UQLGK1qFZ84SD1/PgCwQrKY= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-63-Z64Z0_4eNx2NjwLo5rkHHQ-1; Tue, 05 Apr 2022 08:43:04 -0400 X-MC-Unique: Z64Z0_4eNx2NjwLo5rkHHQ-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E03EA1C0E347; Tue, 5 Apr 2022 12:43:03 +0000 (UTC) Received: from dceara.remote.csb (unknown [10.39.195.57]) by smtp.corp.redhat.com (Postfix) with ESMTP id 858244047272; Tue, 5 Apr 2022 12:43:02 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Tue, 5 Apr 2022 14:43:00 +0200 Message-Id: <20220405124257.16152.15121.stgit@dceara.remote.csb> In-Reply-To: <20220405124203.16152.39687.stgit@dceara.remote.csb> References: <20220405124203.16152.39687.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.11.54.2 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=dceara@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Cc: noloader@gmail.com, i.maximets@ovn.org Subject: [ovs-dev] [PATCH v5 3/6] ofp-actions: Ensure aligned accesses to masked fields. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" For example is parsing the OVN "eth.dst[40] = 1;" action, which triggered the following warning from UndefinedBehaviorSanitizer: lib/meta-flow.c:3210:9: runtime error: member access within misaligned address 0x000000de4e36 for type 'const union mf_value', which requires 8 byte alignment 0x000000de4e36: note: pointer points here 00 00 00 00 01 00 00 00 00 00 00 00 00 00 70 4e de 00 00 00 00 00 10 51 de 00 00 00 00 00 c0 4f ^ #0 0x5818bc in mf_format lib/meta-flow.c:3210 #1 0x5b6047 in format_SET_FIELD lib/ofp-actions.c:3342 #2 0x5d68ab in ofpact_format lib/ofp-actions.c:9213 #3 0x5d6ee0 in ofpacts_format lib/ofp-actions.c:9237 #4 0x410922 in test_parse_actions tests/test-ovn.c:1360 To avoid this we now change the internal representation of the set_field actions, struct ofpact_set_field, such that the mask is always stored at a correctly aligned address, multiple of OFPACT_ALIGNTO. We also need to adapt the "ovs-ofctl show-flows - Oversized flow" test because now the ofpact representation of the set_field action uses more bytes in memory (for the extra alignment). Change the test to use dec_ttl instead. Signed-off-by: Dumitru Ceara Acked-by: Aaron Conole --- v5: Implemented Adrian's suggestion to always align the 'mask' within 'struct ofpact_set_field' to 8 bytes. v4: Rebase. v3: Split out from old patch 07/11. --- include/openvswitch/ofp-actions.h | 10 ++++++---- lib/ofp-actions.c | 19 +++++++++++++------ tests/ovs-ofctl.at | 2 +- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h index b7231c7bb334..711e7c773d6c 100644 --- a/include/openvswitch/ofp-actions.h +++ b/include/openvswitch/ofp-actions.h @@ -549,7 +549,8 @@ struct ofpact_set_field { const struct mf_field *field; ); union mf_value value[]; /* Significant value bytes followed by - * significant mask bytes. */ + * significant mask bytes aligned at + * OFPACT_ALIGNTO bytes. */ }; BUILD_ASSERT_DECL(offsetof(struct ofpact_set_field, value) % OFPACT_ALIGNTO == 0); @@ -557,9 +558,10 @@ BUILD_ASSERT_DECL(offsetof(struct ofpact_set_field, value) == sizeof(struct ofpact_set_field)); /* Use macro to not have to deal with constness. */ -#define ofpact_set_field_mask(SF) \ - ALIGNED_CAST(union mf_value *, \ - (uint8_t *)(SF)->value + (SF)->field->n_bytes) +#define ofpact_set_field_mask(SF) \ + ALIGNED_CAST(union mf_value *, \ + (uint8_t *)(SF)->value + \ + ROUND_UP((SF)->field->n_bytes, OFPACT_ALIGNTO)) /* OFPACT_PUSH_VLAN/MPLS/PBB * diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 4ada0f4a3c49..783af84439e2 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -3376,21 +3376,28 @@ check_SET_FIELD(struct ofpact_set_field *a, return 0; } -/* Appends an OFPACT_SET_FIELD ofpact with enough space for the value and mask - * for the 'field' to 'ofpacts' and returns it. Fills in the value from - * 'value', if non-NULL, and mask from 'mask' if non-NULL. If 'value' is - * non-NULL and 'mask' is NULL, an all-ones mask will be filled in. */ +/* Appends an OFPACT_SET_FIELD ofpact with enough space for the value and a + * properly aligned mask for the 'field' to 'ofpacts' and returns it. Fills + * in the value from 'value', if non-NULL, and mask from 'mask' if non-NULL. + * If 'value' is non-NULL and 'mask' is NULL, an all-ones mask will be + * filled in. */ struct ofpact_set_field * ofpact_put_set_field(struct ofpbuf *ofpacts, const struct mf_field *field, const void *value, const void *mask) { + /* Ensure there's enough space for: + * - value (8-byte aligned) + * - mask (8-byte aligned) + * - padding (to make the whole ofpact_set_field 8-byte aligned) + */ + size_t total_size = 2 * ROUND_UP(field->n_bytes, OFPACT_ALIGNTO); struct ofpact_set_field *sf = ofpact_put_SET_FIELD(ofpacts); sf->field = field; /* Fill in the value and mask if given, otherwise put zeroes so that the * caller may fill in the value and mask itself. */ if (value) { - ofpbuf_put_uninit(ofpacts, 2 * field->n_bytes); + ofpbuf_put_uninit(ofpacts, total_size); sf = ofpacts->header; memcpy(sf->value, value, field->n_bytes); if (mask) { @@ -3399,7 +3406,7 @@ ofpact_put_set_field(struct ofpbuf *ofpacts, const struct mf_field *field, memset(ofpact_set_field_mask(sf), 0xff, field->n_bytes); } } else { - ofpbuf_put_zeros(ofpacts, 2 * field->n_bytes); + ofpbuf_put_zeros(ofpacts, total_size); sf = ofpacts->header; } /* Update length. */ diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index 267711bfa482..858dcda1f347 100644 --- a/tests/ovs-ofctl.at +++ b/tests/ovs-ofctl.at @@ -3258,7 +3258,7 @@ AT_SETUP([ovs-ofctl show-flows - Oversized flow]) OVS_VSWITCHD_START printf " priority=90,icmp,reg15=0x8005,metadata=0x1,nw_dst=11.0.0.1,icmp_type=8,icmp_code=0 actions=" > flow.txt -for i in `seq 1 1022`; do printf "set_field:0x399->reg13,set_field:0x$i->reg15,resubmit(,39),"; done >> flow.txt +for i in `seq 1 2045`; do printf "dec_ttl,resubmit(,39),"; done >> flow.txt printf "resubmit(,39)\n" >> flow.txt AT_CHECK([ovs-ofctl -O OpenFlow15 add-flows br0 flow.txt])