When connecting two LRPs directly with each other the LRP->peer value is
set to the LRP->name value of the respective other LRP.
If we have LRP1 and LRP2. Previously LRP1->peer = LRP2 && LRP2->peer = ""
would have been processed by northd as if LRP1 was connect to LRP2 but not
the other way round.
Additionally it was possible to set
LRP1->peer = LRP2 && LRP2->peer = LRP3 && LRP3->peer = LRP1
Both of these options are invalid and in the past have produced
use-after-frees in northd, as reported by ASAN below.
The issue is present at least back until 2bdf1129c1. But probably it is
older than that.
==845947==ERROR: AddressSanitizer: heap-use-after-free on address
0x5120000060f8 at pc 0x585dd65a92ab bp 0x7fffffc7fab0 sp 0x7fffffc7faa8
WRITE of size 8 at 0x5120000060f8 thread T0
#0 0x585dd65a92aa in ovn_port_cleanup [...]/ovn/northd/northd.c:1240:26
#1 0x585dd65a868c in ovn_port_destroy_orphan
[...]/ovn/northd/northd.c:1257:5
#2 0x585dd65ac254 in ovn_port_destroy [...]/ovn/northd/northd.c:1276:9
#3 0x585dd658d602 in destroy_datapaths_and_ports
[...]/ovn/northd/northd.c:18362:9
#4 0x585dd658ccad in northd_destroy [...]/ovn/northd/northd.c:18443:5
#5 0x585dd66b56b3 in en_northd_run [...]/ovn/northd/en-northd.c:122:5
#6 0x585dd6780411 in engine_recompute [...]/ovn/lib/inc-proc-eng.c:430:5
#7 0x585dd6781796 in engine_compute [...]/ovn/lib/inc-proc-eng.c:469:17
#8 0x585dd677e75e in engine_run_node [...]/ovn/lib/inc-proc-eng.c:518:14
#9 0x585dd677d871 in engine_run [...]/ovn/lib/inc-proc-eng.c:543:9
#10 0x585dd6719cf3 in inc_proc_northd_run
[...]/ovn/northd/inc-proc-northd.c:470:5
#11 0x585dd66a13e5 in main [...]/ovn/northd/ovn-northd.c:1055:32
#12 0x7822ced45e07 in __libc_start_call_main
/usr/src/debug/glibc/glibc/csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#13 0x7822ced45ecb in __libc_start_main
/usr/src/debug/glibc/glibc/csu/../csu/libc-start.c:360:3
#14 0x585dd6409d74 in _start ([...]/ovn/northd/ovn-northd+0x9cad74)
(BuildId: 7943a43509efc7074c04a1d7d0a37692f00fa1c4)
0x5120000060f8 is located 184 bytes inside of 312-byte region
[0x512000006040,0x512000006178)
freed by thread T0 here:
#0 0x585dd64f4eb2 in free.part.0 asan_malloc_linux.cpp.o
#1 0x585dd65a88f3 in ovn_port_destroy_orphan
[...]/ovn/northd/northd.c:1263:5
#2 0x585dd65ac254 in ovn_port_destroy [...]/ovn/northd/northd.c:1276:9
#3 0x585dd658d602 in destroy_datapaths_and_ports
[...]/ovn/northd/northd.c:18362:9
#4 0x585dd658ccad in northd_destroy [...]/ovn/northd/northd.c:18443:5
#5 0x585dd66b56b3 in en_northd_run [...]/ovn/northd/en-northd.c:122:5
#6 0x585dd6780411 in engine_recompute [...]/ovn/lib/inc-proc-eng.c:430:5
#7 0x585dd6781796 in engine_compute [...]/ovn/lib/inc-proc-eng.c:469:17
#8 0x585dd677e75e in engine_run_node [...]/ovn/lib/inc-proc-eng.c:518:14
#9 0x585dd677d871 in engine_run [...]/ovn/lib/inc-proc-eng.c:543:9
#10 0x585dd6719cf3 in inc_proc_northd_run
[...]/ovn/northd/inc-proc-northd.c:470:5
#11 0x585dd66a13e5 in main [...]/ovn/northd/ovn-northd.c:1055:32
#12 0x7822ced45e07 in __libc_start_call_main
/usr/src/debug/glibc/glibc/csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#13 0x7822ced45ecb in __libc_start_main
/usr/src/debug/glibc/glibc/csu/../csu/libc-start.c:360:3
#14 0x585dd6409d74 in _start ([...]/ovn/northd/ovn-northd+0x9cad74)
(BuildId: 7943a43509efc7074c04a1d7d0a37692f00fa1c4)
previously allocated by thread T0 here:
#0 0x585dd64f6199 in calloc ([...]/ovn/northd/ovn-northd+0xab7199)
(BuildId: 7943a43509efc7074c04a1d7d0a37692f00fa1c4)
#1 0x585dd6df15e2 in xcalloc__ [...]/ovn/ovs/lib/util.c:125:31
#2 0x585dd6df16b4 in xzalloc__ [...]/ovn/ovs/lib/util.c:135:12
#3 0x585dd6df1a19 in xzalloc [...]/ovn/ovs/lib/util.c:169:12
#4 0x585dd65ac63c in ovn_port_create [...]/ovn/northd/northd.c:1207:27
#5 0x585dd667d344 in join_logical_ports [...]/ovn/northd/northd.c:2337:31
#6 0x585dd659540e in build_ports [...]/ovn/northd/northd.c:4236:5
#7 0x585dd658f77d in ovnnb_db_run [...]/ovn/northd/northd.c:18539:5
#8 0x585dd66b5835 in en_northd_run [...]/ovn/northd/en-northd.c:129:5
#9 0x585dd6780411 in engine_recompute [...]/ovn/lib/inc-proc-eng.c:430:5
#10 0x585dd6781796 in engine_compute [...]/ovn/lib/inc-proc-eng.c:469:17
#11 0x585dd677e75e in engine_run_node [...]/ovn/lib/inc-proc-eng.c:518:14
#12 0x585dd677d871 in engine_run [...]/ovn/lib/inc-proc-eng.c:543:9
#13 0x585dd6719cf3 in inc_proc_northd_run
[...]/ovn/northd/inc-proc-northd.c:470:5
#14 0x585dd66a13e5 in main [...]/ovn/northd/ovn-northd.c:1055:32
#15 0x7822ced45e07 in __libc_start_call_main
/usr/src/debug/glibc/glibc/csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#16 0x7822ced45ecb in __libc_start_main
/usr/src/debug/glibc/glibc/csu/../csu/libc-start.c:360:3
#17 0x585dd6409d74 in _start ([...]/ovn/northd/ovn-northd+0x9cad74)
(BuildId: 7943a43509efc7074c04a1d7d0a37692f00fa1c4)
Signed-off-by: Felix Huettner <felix.huettner@stackit.cloud>
---
northd/northd.c | 5 ++++-
tests/ovn-northd.at | 47 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 51 insertions(+), 1 deletion(-)
diff --git a/northd/northd.c b/northd/northd.c
index 587fcd586..d70d0db4b 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -2426,7 +2426,10 @@ join_logical_ports(const struct sbrec_port_binding_table
*sbrec_pb_table,
} else if (op->nbrp && op->nbrp->peer && !is_cr_port(op)) {
struct ovn_port *peer = ovn_port_find(ports, op->nbrp->peer);
if (peer) {
- if (peer->nbrp) {
+ if (peer->nbrp && peer->nbrp->peer &&
+ !strcmp(op->nbrp->name, peer->nbrp->peer)) {
+ /* We only configure LRP peers if each LRP has the other as
+ * its peer. */
op->peer = peer;
} else if (peer->nbsp) {
/* An ovn_port for a switch port of type "router" does have
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index d486f042b..29f1a1f11 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -14630,3 +14630,50 @@ check_row_count Port_Binding 1 logical_port=tr-az4
type=patch
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([configure LRP peers strange])
+ovn_start
+
+check ovn-nbctl lr-add lr0
+check ovn-nbctl lr-add lr1
+check ovn-nbctl lr-add lr2
+check ovn-nbctl lrp-add lr0 lrp0 00:00:00:02:ff:01 10.10.0.1/24
+check ovn-nbctl lrp-add lr1 lrp1 00:00:00:02:ff:02 10.10.0.2/24
+check ovn-nbctl lrp-add lr2 lrp2 00:00:00:02:ff:03 10.10.0.3/24
+check ovn-nbctl --wait=sb sync
+
+# We test here that we can configure lrp peers after the lrps have been
+# created. We use multiple waits to ensure that northd needs to handle all
+# partial states.
+check_row_count Port_Binding 1 logical_port=lrp0 options:peer{not-in}lrp1
+check_row_count Port_Binding 1 logical_port=lrp1 options:peer{not-in}lrp0
+
+check ovn-nbctl --wait=sb set Logical_Router_Port lrp0 peer=lrp1
+check_row_count Port_Binding 1 logical_port=lrp0 options:peer{not-in}lrp1
+check_row_count Port_Binding 1 logical_port=lrp1 options:peer{not-in}lrp0
+
+check ovn-nbctl --wait=sb set Logical_Router_Port lrp1 peer=lrp0
+check_row_count Port_Binding 1 logical_port=lrp0 options:peer=lrp1
+check_row_count Port_Binding 1 logical_port=lrp1 options:peer=lrp0
+
+# Now we use 3 LRPs on different LRs and configure the peers in a circle so
+# that lrp0->lrp1->lrp2->lrp0. While this setup is obviously not valid we
+# should not crash.
+check ovn-nbctl --wait=sb clear Logical_Router_Port lrp1 peer
+check_row_count Port_Binding 1 logical_port=lrp0 options:peer{not-in}lrp1
+check_row_count Port_Binding 1 logical_port=lrp1 options:peer{not-in}lrp2
+check_row_count Port_Binding 1 logical_port=lrp2 options:peer{not-in}lrp0
+
+check ovn-nbctl --wait=sb set Logical_Router_Port lrp1 peer=lrp2
+check_row_count Port_Binding 1 logical_port=lrp0 options:peer{not-in}lrp1
+check_row_count Port_Binding 1 logical_port=lrp1 options:peer{not-in}lrp2
+check_row_count Port_Binding 1 logical_port=lrp2 options:peer{not-in}lrp0
+
+check ovn-nbctl --wait=sb set Logical_Router_Port lrp2 peer=lrp0
+check_row_count Port_Binding 1 logical_port=lrp0 options:peer{not-in}lrp1
+check_row_count Port_Binding 1 logical_port=lrp1 options:peer{not-in}lrp2
+check_row_count Port_Binding 1 logical_port=lrp2 options:peer{not-in}lrp0
+
+AT_CLEANUP
+])
base-commit: 7c3f7f415f1de65dfaacb86836de51cb85bfb0b0
--
2.47.1
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
