Skip to content

Commit ea3667c

Browse files
committed
ovsdb-idl: Fix returning non-existent rows from uuid lookup.
IDL may contain deleted or orphan rows that should never be visible to the user. However, ovsdb_idl_get_row_for_uuid() function simply looks up the row by UUID and returns it without checking if the row actually exists in the IDL. This is causing a crash on assertion failure in ovn-controller when it looks up and finds port binding records that were already deleted: 5 vlog_abort at lib/vlog.c:1325 6 ovs_assert_failure at lib/util.c:90 7 ovsdb_idl_txn_write__.constprop.0 at lib/ovsdb-idl.c:3650 8 ovsdb_idl_txn_write at lib/ovsdb-idl.c:3742 9 sbrec_port_binding_set_up at lib/ovn-sb-idl.c:39665 10 port_binding_set_down at controller/binding.c:3700 11 if_status_mgr_update at controller/if-status.c:645 12 main at controller/ovn-controller.c:7544 7 ovsdb_idl_txn_write__.constprop.0 at lib/ovsdb-idl.c:3650 3650 ovs_assert(row->new_datum != NULL); Can be easily reproduced with ovs-vsctl: $ ovs-vsctl add-br br-int $ ovs-vsctl del-br br-int \ -- set bridge $(ovs-vsctl get bridge br-int _uuid) other-config:a=b ovs-vsctl: lib/ovsdb-idl.c:2673: assertion row->new_datum != NULL failed in ovsdb_idl_read() Aborted (core dumped) Fix that by adding an extra check for row existence like in IDL iterators, so deleted or orphan rows can no longer be found. Fixes: 979821c ("ovsdb-idl: Allow clients to modify records without using structs.") Reported-by: Dumitru Ceara <[email protected]> Acked-by: Eelco Chaudron <[email protected]> Acked-by: Dumitru Ceara <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>
1 parent 3c60bff commit ea3667c

File tree

2 files changed

+20
-1
lines changed

2 files changed

+20
-1
lines changed

lib/ovsdb-idl.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2549,7 +2549,10 @@ ovsdb_idl_get_row_for_uuid(const struct ovsdb_idl *idl,
25492549
const struct ovsdb_idl_table_class *tc,
25502550
const struct uuid *uuid)
25512551
{
2552-
return ovsdb_idl_get_row(ovsdb_idl_table_from_class(idl, tc), uuid);
2552+
const struct ovsdb_idl_row *row;
2553+
2554+
row = ovsdb_idl_get_row(ovsdb_idl_table_from_class(idl, tc), uuid);
2555+
return (row && ovsdb_idl_row_exists(row)) ? row : NULL;
25532556
}
25542557

25552558
static struct ovsdb_idl_row *

tests/ovs-vsctl.at

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,22 @@ CHECK_PORTS([a])
417417
OVS_VSCTL_CLEANUP
418418
AT_CLEANUP
419419

420+
AT_SETUP([add-br a, del-br a + set external-ids on deleted bridge])
421+
AT_KEYWORDS([ovs-vsctl])
422+
OVS_VSCTL_SETUP
423+
AT_CHECK([RUN_OVS_VSCTL([add-br a])])
424+
AT_CHECK([RUN_OVS_VSCTL_TOGETHER(
425+
[del-br a],
426+
[set bridge $(RUN_OVS_VSCTL([get bridge a _uuid])) external-ids:key0=value0])],
427+
[1], [], [stderr])
428+
AT_CHECK([uuidfilt stderr], [0], [dnl
429+
ovs-vsctl: no row "<0>" in table Bridge
430+
])
431+
CHECK_BRIDGES([a, a, 0])
432+
CHECK_PORTS([a])
433+
OVS_VSCTL_CLEANUP
434+
AT_CLEANUP
435+
420436
AT_SETUP([external IDs])
421437
AT_KEYWORDS([ovs-vsctl])
422438
OVS_VSCTL_SETUP

0 commit comments

Comments
 (0)