This avoids misaligned accesses as flagged by UBSan when running CoPP
system tests:
controller/pinctrl.c:7129:28: runtime error: member access within misaligned
address 0x61b0000627be for type 'const struct bfd_msg', which requires 4 byte
alignment
0x61b0000627be: note: pointer points here
00 20 d3 d4 20 60 05 18 a8 99 e7 7b 92 1d 36 73 00 0f 42 40 00 0f 42 40
00 00 00 00 00 00 00 03
^
#0 0x621b8f in pinctrl_check_bfd_msg
/root/ovn/controller/pinctrl.c:7129:28
#1 0x621b8f in pinctrl_handle_bfd_msg
/root/ovn/controller/pinctrl.c:7183:10
#2 0x621b8f in process_packet_in /root/ovn/controller/pinctrl.c:3320:9
#3 0x621b8f in pinctrl_recv /root/ovn/controller/pinctrl.c:3386:9
#4 0x621b8f in pinctrl_handler /root/ovn/controller/pinctrl.c:3481:17
#5 0xa2d89a in ovsthread_wrapper /root/ovn/ovs/lib/ovs-thread.c:422:12
#6 0x7fcb598081ce in start_thread (/lib64/libpthread.so.0+0x81ce)
#7 0x7fcb58439dd2 in clone (/lib64/libc.so.6+0x39dd2)
Fixes: 117203584d98 ("controller: introduce BFD tx path in ovn-controller.")
Reported-by: Ilya Maximets <[email protected]>
Signed-off-by: Dumitru Ceara <[email protected]>
---
controller/pinctrl.c | 26 ++++++++++++++------------
lib/ovn-l7.h | 10 +++++-----
2 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 8859cb080d..9913d5ae12 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -6890,13 +6890,14 @@ pinctrl_find_bfd_monitor_entry_by_port(char *ip,
uint16_t port)
}
static struct bfd_entry *
-pinctrl_find_bfd_monitor_entry_by_disc(char *ip, ovs_be32 disc)
+pinctrl_find_bfd_monitor_entry_by_disc(const char *ip,
+ const ovs_16aligned_be32 *disc)
{
struct bfd_entry *ret = NULL, *entry;
HMAP_FOR_EACH_WITH_HASH (entry, node, hash_string(ip, 0),
&bfd_monitor_map) {
- if (entry->local_disc == disc) {
+ if (entry->local_disc == get_16aligned_be32(disc)) {
ret = entry;
break;
}
@@ -6955,11 +6956,11 @@ bfd_monitor_put_bfd_msg(struct bfd_entry *entry, struct
dp_packet *packet,
msg->length = BFD_PACKET_LEN;
msg->flags = final ? BFD_FLAG_FINAL : 0;
msg->flags |= entry->state << 6;
- msg->my_disc = entry->local_disc;
- msg->your_disc = entry->remote_disc;
+ put_16aligned_be32(&msg->my_disc, entry->local_disc);
+ put_16aligned_be32(&msg->your_disc, entry->remote_disc);
/* min_tx and min_rx are in us - RFC 5880 page 9 */
- msg->min_tx = htonl(entry->local_min_tx * 1000);
- msg->min_rx = htonl(entry->local_min_rx * 1000);
+ put_16aligned_be32(&msg->min_tx, htonl(entry->local_min_tx * 1000));
+ put_16aligned_be32(&msg->min_rx, htonl(entry->local_min_rx * 1000));
if (!IN6_IS_ADDR_V4MAPPED(&entry->ip_src)) {
/* IPv6 needs UDP checksum calculated */
@@ -7154,7 +7155,7 @@ pinctrl_check_bfd_msg(const struct flow *ip_flow, struct
dp_packet *pkt_in)
return false;
}
- if (!msg->my_disc) {
+ if (!get_16aligned_be32(&msg->my_disc)) {
VLOG_DBG_RL(&rl, "BFD action: my_disc not set");
return false;
}
@@ -7165,7 +7166,8 @@ pinctrl_check_bfd_msg(const struct flow *ip_flow, struct
dp_packet *pkt_in)
}
enum bfd_state peer_state = msg->flags >> 6;
- if (peer_state >= BFD_STATE_INIT && !msg->your_disc) {
+ if (peer_state >= BFD_STATE_INIT
+ && !get_16aligned_be32(&msg->your_disc)) {
VLOG_DBG_RL(&rl,
"BFD action: remote state is %s and your_disc is not set",
bfd_get_status(peer_state));
@@ -7193,7 +7195,7 @@ pinctrl_handle_bfd_msg(struct rconn *swconn, const struct
flow *ip_flow,
const struct bfd_msg *msg = dp_packet_get_udp_payload(pkt_in);
struct bfd_entry *entry =
- pinctrl_find_bfd_monitor_entry_by_disc(ip_src, msg->your_disc);
+ pinctrl_find_bfd_monitor_entry_by_disc(ip_src, &msg->your_disc);
if (!entry) {
free(ip_src);
@@ -7201,9 +7203,9 @@ pinctrl_handle_bfd_msg(struct rconn *swconn, const struct
flow *ip_flow,
}
bool change_state = false;
- entry->remote_disc = msg->my_disc;
- uint32_t remote_min_tx = ntohl(msg->min_tx) / 1000;
- entry->remote_min_rx = ntohl(msg->min_rx) / 1000;
+ entry->remote_disc = get_16aligned_be32(&msg->my_disc);
+ uint32_t remote_min_tx = ntohl(get_16aligned_be32(&msg->min_tx)) / 1000;
+ entry->remote_min_rx = ntohl(get_16aligned_be32(&msg->min_rx)) / 1000;
entry->detection_timeout = msg->mult * MAX(remote_min_tx,
entry->local_min_rx);
diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
index b03b945d89..2b20bc380a 100644
--- a/lib/ovn-l7.h
+++ b/lib/ovn-l7.h
@@ -37,11 +37,11 @@ struct bfd_msg {
uint8_t flags;
uint8_t mult;
uint8_t length;
- ovs_be32 my_disc;
- ovs_be32 your_disc;
- ovs_be32 min_tx;
- ovs_be32 min_rx;
- ovs_be32 min_rx_echo;
+ ovs_16aligned_be32 my_disc;
+ ovs_16aligned_be32 your_disc;
+ ovs_16aligned_be32 min_tx;
+ ovs_16aligned_be32 min_rx;
+ ovs_16aligned_be32 min_rx_echo;
};
BUILD_ASSERT_DECL(BFD_PACKET_LEN == sizeof(struct bfd_msg));
--
2.31.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
