diff mbox series

[ovs-dev,08/11] bfd: lldp: stp: Fix misaligned packet field access.

Message ID 20211217131817.30994.11565.stgit@dceara.remote.csb
State Superseded
Headers show
Series Fix UndefinedBehaviorSanitizer reported issues and enable it in CI. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Dumitru Ceara Dec. 17, 2021, 1:18 p.m. UTC
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 <dceara@redhat.com>
---
 lib/bfd.c       |   51 ++++++++++++++++++++++++++++-----------------------
 lib/lldp/lldp.c |    4 +++-
 lib/stp.c       |   16 ++++++++--------
 3 files changed, 39 insertions(+), 32 deletions(-)
diff mbox series

Patch

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;
     }