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: 979821c0a6b0 ("ovsdb-idl: Allow clients to modify records without using structs.")
Reported-by: Dumitru Ceara <dceara@redhat.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 7f49e922d356972dd6dc943bb976b1fdbaed6f88..d8094458dc73d233d195a1bca1d60189e2a8943e 100644 (file)
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -2602,7 +2602,10 @@ ovsdb_idl_get_row_for_uuid(const struct ovsdb_idl *idl,
                            const struct ovsdb_idl_table_class *tc,
                            const struct uuid *uuid)
 {
-    return ovsdb_idl_get_row(ovsdb_idl_table_from_class(idl, tc), uuid);
+    const struct ovsdb_idl_row *row;
+
+    row = ovsdb_idl_get_row(ovsdb_idl_table_from_class(idl, tc), uuid);
+    return (row && ovsdb_idl_row_exists(row)) ? row : NULL;
 }
 
 static struct ovsdb_idl_row *

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index abc102510b444adf908ce8fa38dbce884512b491..4ca5261c2382cea585d7f5534fbce5790184495d 100644 (file)
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -417,6 +417,22 @@ CHECK_PORTS([a])
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
+AT_SETUP([add-br a, del-br a + set external-ids on deleted bridge])
+AT_KEYWORDS([ovs-vsctl])
+OVS_VSCTL_SETUP
+AT_CHECK([RUN_OVS_VSCTL([add-br a])])
+AT_CHECK([RUN_OVS_VSCTL_TOGETHER(
+  [del-br a],
+  [set bridge $(RUN_OVS_VSCTL([get bridge a _uuid])) external-ids:key0=value0])],
+  [1], [], [stderr])
+AT_CHECK([uuidfilt stderr], [0], [dnl
+ovs-vsctl: no row "<0>" in table Bridge
+])
+CHECK_BRIDGES([a, a, 0])
+CHECK_PORTS([a])
+OVS_VSCTL_CLEANUP
+AT_CLEANUP
+
 AT_SETUP([external IDs])
 AT_KEYWORDS([ovs-vsctl])
 OVS_VSCTL_SETUP