From patchwork Tue Dec 21 11:03:13 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1571575 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=MyNsDoIA; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=2605:bc80:3010::138; helo=smtp1.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Received: from smtp1.osuosl.org (smtp1.osuosl.org [IPv6:2605:bc80:3010::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 4JJD915mrkz9s0r for ; Tue, 21 Dec 2021 22:03:49 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 776C782894; Tue, 21 Dec 2021 11:03:47 +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 TebB1SWqcD0Z; Tue, 21 Dec 2021 11:03:46 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp1.osuosl.org (Postfix) with ESMTPS id 786C4826D5; Tue, 21 Dec 2021 11:03:45 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 3E1E3C0038; Tue, 21 Dec 2021 11:03:45 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) by lists.linuxfoundation.org (Postfix) with ESMTP id 1F2D7C0012 for ; Tue, 21 Dec 2021 11:03:44 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id A32E1409F2 for ; Tue, 21 Dec 2021 11:03:22 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp2.osuosl.org (amavisd-new); dkim=pass (1024-bit key) header.d=redhat.com Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ZbLhmg70gPKj for ; Tue, 21 Dec 2021 11:03:21 +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 smtp2.osuosl.org (Postfix) with ESMTPS id 87DCA405D5 for ; Tue, 21 Dec 2021 11:03:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1640084599; 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=BgMFPQc7KijXe8fBpKckcjB538zbc8qOpYWZ2MHYKDU=; b=MyNsDoIA5/9DXIRFLr7ZdPgpKPDC9pKMJzOBIBGmtAyEcKqoh5IS/QBDwgpVoWjEYNUAYv HyGhq1Q371bZF2wjnfBsyZMIUE9PTScSKlwi5zh14ruzvyeM/5ggdJfqD37fO8ihfXnTVp NonwSqLWALFlUVNsDeWHVhkcIDpjOc4= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-551-LDNr_5xrPPO9gkh3VGXsuA-1; Tue, 21 Dec 2021 06:03:18 -0500 X-MC-Unique: LDNr_5xrPPO9gkh3VGXsuA-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 F0B7E1006AA4; Tue, 21 Dec 2021 11:03:16 +0000 (UTC) Received: from dceara.remote.csb (unknown [10.39.193.163]) by smtp.corp.redhat.com (Postfix) with ESMTP id EA6C75BD0A; Tue, 21 Dec 2021 11:03:15 +0000 (UTC) From: Dumitru Ceara To: dev@openvswitch.org Date: Tue, 21 Dec 2021 12:03:13 +0100 Message-Id: <20211221110311.14345.60645.stgit@dceara.remote.csb> In-Reply-To: <20211221110004.14345.44808.stgit@dceara.remote.csb> References: <20211221110004.14345.44808.stgit@dceara.remote.csb> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 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: i.maximets@ovn.org Subject: [ovs-dev] [PATCH v2 08/11] bfd: lldp: stp: Fix misaligned packet field access. 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" UB Sanitizer reports: lib/bfd.c:748:16: runtime error: member access within misaligned address 0x000001f0d6ea for type 'struct msg', which requires 4 byte alignment 0x000001f0d6ea: note: pointer points here 00 20 00 00 20 40 03 18 93 f9 0a 6e 00 00 00 00 00 0f 42 40 00 0f 42 40 00 00 00 00 cc 00 00 00 ^ #0 0x59008e in bfd_process_packet lib/bfd.c:748 #1 0x52a240 in process_special ofproto/ofproto-dpif-xlate.c:3370 #2 0x553452 in xlate_actions ofproto/ofproto-dpif-xlate.c:7766 #3 0x4fc9e6 in upcall_xlate ofproto/ofproto-dpif-upcall.c:1237 #4 0x4fdecc in process_upcall ofproto/ofproto-dpif-upcall.c:1456 #5 0x4fd936 in upcall_cb ofproto/ofproto-dpif-upcall.c:1358 [...] lib/stp.c:754:15: runtime error: member access within misaligned address 0x000002c4ea61 for type 'const struct stp_bpdu_header', which requires 2 byte alignment 0x000002c4ea61: note: pointer points here 26 42 42 03 00 00 00 00 00 80 00 aa 66 aa 66 00 01 00 00 00 00 80 00 aa 66 aa 66 00 01 80 02 00 ^ #0 0x8a2bce in stp_received_bpdu lib/stp.c:754 #1 0x51e603 in stp_process_packet ofproto/ofproto-dpif-xlate.c:1788 #2 0x52a96d in process_special ofproto/ofproto-dpif-xlate.c:3394 #3 0x5534df in xlate_actions ofproto/ofproto-dpif-xlate.c:7766 #4 0x4fcb49 in upcall_xlate ofproto/ofproto-dpif-upcall.c:1237 [...] lib/lldp/lldp.c:149:10: runtime error: load of misaligned address 0x7ffcc0ae72bd for type 'ovs_be16', which requires 2 byte alignment 0x7ffcc0ae72bd: note: pointer points here 8e e7 84 ad 04 00 05 46 61 73 74 45 74 68 65 72 6e 65 74 20 31 2f 35 e0 91 07 c9 3e 7f 00 00 80 ^ #0 0x718d63 in lldp_tlv_end lib/lldp/lldp.c:149 #1 0x7191de in lldp_send lib/lldp/lldp.c:184 #2 0x484d6c in test_aa_send tests/test-aa.c:238 [...] Signed-off-by: Dumitru Ceara Acked-by: Aaron Conole --- lib/bfd.c | 51 ++++++++++++++++++++++++++++----------------------- lib/lldp/lldp.c | 4 +++- lib/stp.c | 16 ++++++++-------- 3 files changed, 39 insertions(+), 32 deletions(-) diff --git a/lib/bfd.c b/lib/bfd.c index 3c965699ace3..9698576d071b 100644 --- a/lib/bfd.c +++ b/lib/bfd.c @@ -131,16 +131,17 @@ enum diag { * | Required Min Echo RX Interval | * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ */ struct msg { - uint8_t vers_diag; /* Version and diagnostic. */ - uint8_t flags; /* 2bit State field followed by flags. */ - uint8_t mult; /* Fault detection multiplier. */ - uint8_t length; /* Length of this BFD message. */ - ovs_be32 my_disc; /* My discriminator. */ - ovs_be32 your_disc; /* Your discriminator. */ - ovs_be32 min_tx; /* Desired minimum tx interval. */ - ovs_be32 min_rx; /* Required minimum rx interval. */ - ovs_be32 min_rx_echo; /* Required minimum echo rx interval. */ + uint8_t vers_diag; /* Version and diagnostic. */ + uint8_t flags; /* 2bit State field followed by flags. */ + uint8_t mult; /* Fault detection multiplier. */ + uint8_t length; /* Length of this BFD message. */ + ovs_16aligned_be32 my_disc; /* My discriminator. */ + ovs_16aligned_be32 your_disc; /* Your discriminator. */ + ovs_16aligned_be32 min_tx; /* Desired minimum tx interval. */ + ovs_16aligned_be32 min_rx; /* Required minimum rx interval. */ + ovs_16aligned_be32 min_rx_echo; /* Required minimum echo rx interval. */ }; + BUILD_ASSERT_DECL(BFD_PACKET_LEN == sizeof(struct msg)); #define DIAG_MASK 0x1f @@ -634,9 +635,9 @@ bfd_put_packet(struct bfd *bfd, struct dp_packet *p, msg->mult = bfd->mult; msg->length = BFD_PACKET_LEN; - msg->my_disc = htonl(bfd->disc); - msg->your_disc = htonl(bfd->rmt_disc); - msg->min_rx_echo = htonl(0); + put_16aligned_be32(&msg->my_disc, htonl(bfd->disc)); + put_16aligned_be32(&msg->your_disc, htonl(bfd->rmt_disc)); + put_16aligned_be32(&msg->min_rx_echo, htonl(0)); if (bfd_in_poll(bfd)) { min_tx = bfd->poll_min_tx; @@ -646,8 +647,8 @@ bfd_put_packet(struct bfd *bfd, struct dp_packet *p, min_rx = bfd->min_rx; } - msg->min_tx = htonl(min_tx * 1000); - msg->min_rx = htonl(min_rx * 1000); + put_16aligned_be32(&msg->min_tx, htonl(min_tx * 1000)); + put_16aligned_be32(&msg->min_rx, htonl(min_rx * 1000)); bfd->flags &= ~FLAG_FINAL; *oam = bfd->oam; @@ -781,12 +782,12 @@ bfd_process_packet(struct bfd *bfd, const struct flow *flow, goto out; } - if (!msg->my_disc) { + if (!get_16aligned_be32(&msg->my_disc)) { log_msg(VLL_WARN, msg, "NULL my_disc", bfd); goto out; } - pkt_your_disc = ntohl(msg->your_disc); + pkt_your_disc = ntohl(get_16aligned_be32(&msg->your_disc)); if (pkt_your_disc) { /* Technically, we should use the your discriminator field to figure * out which 'struct bfd' this packet is destined towards. That way a @@ -806,7 +807,7 @@ bfd_process_packet(struct bfd *bfd, const struct flow *flow, bfd_status_changed(bfd); } - bfd->rmt_disc = ntohl(msg->my_disc); + bfd->rmt_disc = ntohl(get_16aligned_be32(&msg->my_disc)); bfd->rmt_state = rmt_state; bfd->rmt_flags = flags; bfd->rmt_diag = msg->vers_diag & DIAG_MASK; @@ -834,7 +835,7 @@ bfd_process_packet(struct bfd *bfd, const struct flow *flow, bfd->rmt_mult = msg->mult; } - rmt_min_rx = MAX(ntohl(msg->min_rx) / 1000, 1); + rmt_min_rx = MAX(ntohl(get_16aligned_be32(&msg->min_rx)) / 1000, 1); if (bfd->rmt_min_rx != rmt_min_rx) { bfd->rmt_min_rx = rmt_min_rx; if (bfd->next_tx) { @@ -843,7 +844,7 @@ bfd_process_packet(struct bfd *bfd, const struct flow *flow, log_msg(VLL_INFO, msg, "New remote min_rx", bfd); } - bfd->rmt_min_tx = MAX(ntohl(msg->min_tx) / 1000, 1); + bfd->rmt_min_tx = MAX(ntohl(get_16aligned_be32(&msg->min_tx)) / 1000, 1); bfd->detect_time = bfd_rx_interval(bfd) * bfd->rmt_mult + time_msec(); if (bfd->state == STATE_ADMIN_DOWN) { @@ -1105,10 +1106,14 @@ log_msg(enum vlog_level level, const struct msg *p, const char *message, bfd_diag_str(p->vers_diag & DIAG_MASK), bfd_state_str(p->flags & STATE_MASK), p->mult, p->length, bfd_flag_str(p->flags & FLAGS_MASK), - ntohl(p->my_disc), ntohl(p->your_disc), - ntohl(p->min_tx), ntohl(p->min_tx) / 1000, - ntohl(p->min_rx), ntohl(p->min_rx) / 1000, - ntohl(p->min_rx_echo), ntohl(p->min_rx_echo) / 1000); + ntohl(get_16aligned_be32(&p->my_disc)), + ntohl(get_16aligned_be32(&p->your_disc)), + ntohl(get_16aligned_be32(&p->min_tx)), + ntohl(get_16aligned_be32(&p->min_tx)) / 1000, + ntohl(get_16aligned_be32(&p->min_rx)), + ntohl(get_16aligned_be32(&p->min_rx)) / 1000, + ntohl(get_16aligned_be32(&p->min_rx_echo)), + ntohl(get_16aligned_be32(&p->min_rx_echo)) / 1000); bfd_put_details(&ds, bfd); VLOG(level, "%s", ds_cstr(&ds)); ds_destroy(&ds); diff --git a/lib/lldp/lldp.c b/lib/lldp/lldp.c index 18afbab9a7fe..dfeb2a800244 100644 --- a/lib/lldp/lldp.c +++ b/lib/lldp/lldp.c @@ -146,7 +146,9 @@ static void lldp_tlv_end(struct dp_packet *p, unsigned int start) { ovs_be16 *tlv = dp_packet_at_assert(p, start, 2); - *tlv |= htons((dp_packet_size(p) - (start + 2)) & 0x1ff); + put_unaligned_be16(tlv, + get_unaligned_be16(tlv) + | htons((dp_packet_size(p) - (start + 2)) & 0x1ff)); } int diff --git a/lib/stp.c b/lib/stp.c index 809b405a5298..a869b5f390ce 100644 --- a/lib/stp.c +++ b/lib/stp.c @@ -737,7 +737,7 @@ void stp_received_bpdu(struct stp_port *p, const void *bpdu, size_t bpdu_size) { struct stp *stp = p->stp; - const struct stp_bpdu_header *header; + struct stp_bpdu_header header; ovs_mutex_lock(&mutex); if (p->state == STP_DISABLED) { @@ -750,19 +750,19 @@ stp_received_bpdu(struct stp_port *p, const void *bpdu, size_t bpdu_size) goto out; } - header = bpdu; - if (header->protocol_id != htons(STP_PROTOCOL_ID)) { + memcpy(&header, bpdu, sizeof header); + if (header.protocol_id != htons(STP_PROTOCOL_ID)) { VLOG_WARN("%s: received BPDU with unexpected protocol ID %"PRIu16, - stp->name, ntohs(header->protocol_id)); + stp->name, ntohs(header.protocol_id)); p->error_count++; goto out; } - if (header->protocol_version != STP_PROTOCOL_VERSION) { + if (header.protocol_version != STP_PROTOCOL_VERSION) { VLOG_DBG("%s: received BPDU with unexpected protocol version %"PRIu8, - stp->name, header->protocol_version); + stp->name, header.protocol_version); } - switch (header->bpdu_type) { + switch (header.bpdu_type) { case STP_TYPE_CONFIG: if (bpdu_size < sizeof(struct stp_config_bpdu)) { VLOG_WARN("%s: received config BPDU with invalid size %"PRIuSIZE, @@ -785,7 +785,7 @@ stp_received_bpdu(struct stp_port *p, const void *bpdu, size_t bpdu_size) default: VLOG_WARN("%s: received BPDU of unexpected type %"PRIu8, - stp->name, header->bpdu_type); + stp->name, header.bpdu_type); p->error_count++; goto out; }