Skip to content

Commit

Permalink
bfd: lldp: stp: Fix misaligned packet field access.
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@dceara @igsilya
dceara authored and igsilya committed Feb 14, 2022
1 parent b9e8354 commit 172d8bf
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 32 deletions.
51 changes: 28 additions & 23 deletions lib/bfd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion lib/lldp/lldp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions lib/stp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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,
Expand All @@ -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;
}
Expand Down

0 comments on commit 172d8bf

Please sign in to comment.