From 172d8bfed865559986d44aa32832363edcf6d4cd Mon Sep 17 00:00:00 2001 From: Dumitru Ceara Date: Mon, 24 Jan 2022 15:18:36 +0100 Subject: 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 Signed-off-by: Dumitru Ceara Signed-off-by: Ilya Maximets --- lib/bfd.c | 51 ++++++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 23 deletions(-) (limited to 'lib/bfd.c') 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); -- cgit v1.2.1