bfd: lldp: stp: Fix misaligned packet field access.

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 ...
                ^
      #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 ...
                ^
      #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 ...
               ^
      #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
      [...]

Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

1 files changed, 28 insertions, 23 deletions

diff --git a/lib/bfd.c b/lib/bfd.c
index 3c965699a..9698576d0 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);