Initial change set is preserved for as long as the monitor itself.
However, if a new client has a condition on a column that is not
one of the monitored columns, this column will be added to the
monitor via ovsdb_monitor_condition_bind(). This new column, however,
doesn't exist in the initial change set. That will cause ovsdb-server
to malfunction or crash trying to access non-existent column during
condition evaluation:
ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 4 at 0x606000006780 thread T0
0 ovsdb_clause_evaluate ovsdb/condition.c:328:26
1 ovsdb_condition_match_any_clause ovsdb/condition.c:441:13
2 ovsdb_condition_empty_or_match_any ovsdb/condition.h:84:13
3 ovsdb_monitor_row_update_type_condition ovsdb/monitor.c:892:28
4 ovsdb_monitor_compose_row_update2 ovsdb/monitor.c:1058:12
5 ovsdb_monitor_compose_update ovsdb/monitor.c:1172:24
6 ovsdb_monitor_get_update ovsdb/monitor.c:1276:24
7 ovsdb_jsonrpc_monitor_create ovsdb/jsonrpc-server.c:1505:12
8 ovsdb_jsonrpc_session_got_request ovsdb/jsonrpc-server.c:1030:21
9 ovsdb_jsonrpc_session_run ovsdb/jsonrpc-server.c:572:17
10 ovsdb_jsonrpc_session_run_all ovsdb/jsonrpc-server.c:602:21
11 ovsdb_jsonrpc_server_run ovsdb/jsonrpc-server.c:417:9
12 main_loop ovsdb/ovsdb-server.c:222:9
13 main ovsdb/ovsdb-server.c:500:5
14 __libc_start_call_main
15 __libc_start_main@GLIBC_2.2.5
16 _start (ovsdb/ovsdb-server+0x473034)
Located 0 bytes after 64-byte region [0x606000006740,0x606000006780)
allocated by thread T0 here:
0 malloc (ovsdb/ovsdb-server+0x50dc82)
1 xmalloc__ lib/util.c:140:15
2 xmalloc lib/util.c:175:12
3 clone_monitor_row_data ovsdb/monitor.c:336:12
4 ovsdb_monitor_changes_update ovsdb/monitor.c:1384:23
5 ovsdb_monitor_get_initial ovsdb/monitor.c:1535:21
6 ovsdb_jsonrpc_monitor_create ovsdb/jsonrpc-server.c:1502:9
7 ovsdb_jsonrpc_session_got_request ovsdb/jsonrpc-server.c:1030:21
8 ovsdb_jsonrpc_session_run ovsdb/jsonrpc-server.c:572:17
9 ovsdb_jsonrpc_session_run_all ovsdb/jsonrpc-server.c:602:21
10 ovsdb_jsonrpc_server_run ovsdb/jsonrpc-server.c:417:9
11 main_loop ovsdb/ovsdb-server.c:222:9
12 main ovsdb/ovsdb-server.c:500:5
13 __libc_start_call_main
14 __libc_start_main@GLIBC_2.2.5
15 _start (ovsdb/ovsdb-server+0x473034)
Fix that by destroying the initial change set every time new columns
are added to the monitor. This will trigger re-generation of the
change set and it will contain all the necessary columns afterwards.
Fixes: 07c27226ee96 ("ovsdb: Monitor: Keep and maintain the initial change
set.")
Reported-by: Han Zhou <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
---
ovsdb/monitor.c | 15 +++++++++-
tests/ovsdb-monitor.at | 66 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 80 insertions(+), 1 deletion(-)
diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
index 04dcd2298..4afaa89f4 100644
--- a/ovsdb/monitor.c
+++ b/ovsdb/monitor.c
@@ -478,6 +478,7 @@ ovsdb_monitor_add_column(struct ovsdb_monitor *dbmon,
enum ovsdb_monitor_selection select,
bool monitored)
{
+ struct ovsdb_monitor_change_set *mcs;
struct ovsdb_monitor_table *mt;
struct ovsdb_monitor_column *c;
@@ -488,6 +489,18 @@ ovsdb_monitor_add_column(struct ovsdb_monitor *dbmon,
return column->name;
}
+ mcs = dbmon->init_change_set;
+ if (mcs) {
+ /* A new column is going to be added to the monitor. Existing
+ * initial change set doesn't have it, so can no longer be used.
+ * Initial change set is never used by more than one session at
+ * the same time, so it's safe to destroy it here. */
+ ovs_assert(mcs->n_refs == 1);
+ ovsdb_monitor_json_cache_destroy(dbmon, mcs);
+ ovsdb_monitor_change_set_destroy(mcs);
+ dbmon->init_change_set = NULL;
+ }
+
if (mt->n_columns >= mt->allocated_columns) {
mt->columns = x2nrealloc(mt->columns, &mt->allocated_columns,
sizeof *mt->columns);
@@ -614,7 +627,7 @@ ovsdb_monitor_untrack_change_set(struct ovsdb_monitor
*dbmon,
if (--mcs->n_refs == 0) {
if (mcs == dbmon->init_change_set) {
/* The initial change set should exist as long as the
- * monitor itself. */
+ * monitor doesn't change. */
mcs->n_refs++;
return;
} else if (mcs == dbmon->new_change_set) {
diff --git a/tests/ovsdb-monitor.at b/tests/ovsdb-monitor.at
index 7e1ff64f0..12cd2bc31 100644
--- a/tests/ovsdb-monitor.at
+++ b/tests/ovsdb-monitor.at
@@ -1011,3 +1011,69 @@ row,action,name,number,_version
]], [ignore])
AT_CLEANUP
+AT_SETUP([monitor-cond initial reply with condition on non-monitored column])
+AT_KEYWORDS([ovsdb server monitor monitor-cond positive initial non-monitored])
+
+ordinal_schema > schema
+AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])
+on_exit 'kill `cat ovsdb-server.pid`'
+AT_CAPTURE_FILE([ovsdb-server.log])
+AT_CHECK([ovsdb-server --detach --no-chdir --pidfile \
+ --remote=punix:socket --log-file db], [0], [ignore], [ignore])
+
+dnl Initialize the database content.
+for txn in m4_foreach([txn], [[[["ordinals",
+ {"op": "insert",
+ "table": "ordinals",
+ "row": {"number": 0, "name": "zero"}},
+ {"op": "insert",
+ "table": "ordinals",
+ "row": {"number": 1, "name": "one"}},
+ {"op": "insert",
+ "table": "ordinals",
+ "row": {"number": 2, "name": "two"}}]]]], ['txn' ]); do
+ AT_CHECK([ovsdb-client transact unix:socket "$txn"], [0], [ignore], [ignore])
+done
+
+dnl Start a first client that monitors only the column 'name'.
+on_exit 'kill `cat client-1.pid`'
+AT_CAPTURE_FILE([client-1.out])
+AT_CHECK([ovsdb-client -vjsonrpc --pidfile=client-1.pid --detach --no-chdir \
+ -d json monitor-cond --format=csv unix:socket \
+ ordinals '[[true]]' ordinals ["name"] \
+ > client-1.out 2> client-1.err], [0], [ignore], [ignore])
+dnl Wait for the initial monitor reply.
+OVS_WAIT_UNTIL([grep -q 'initial' client-1.out])
+
+dnl Start a second client that monitors the column 'name', but has a condition
+dnl on column 'number'.
+on_exit 'kill `cat client-2.pid`'
+AT_CAPTURE_FILE([client-2.out])
+AT_CHECK([ovsdb-client -vjsonrpc --pidfile=client-2.pid --detach --no-chdir \
+ -d json monitor-cond --format=csv unix:socket \
+ ordinals '[[["number", "!=", 1]]]' ordinals ["name"] \
+ > client-2.out 2> client-2.err], [0], [ignore], [ignore])
+dnl Wait for the initial monitor reply.
+OVS_WAIT_UNTIL([grep -q 'initial' client-2.out])
+
+OVSDB_SERVER_SHUTDOWN
+OVS_WAIT_UNTIL([test ! -e ovsdb-server.pid && \
+ test ! -e client-1.pid && test ! -e client-2.pid])
+
+dnl The first client should have all the names.
+AT_CHECK([$PYTHON3 $srcdir/ovsdb-monitor-sort.py < client-1.out | uuidfilt],
+ [0], [dnl
+row,action,name
+<0>,initial,"""one"""
+<1>,initial,"""two"""
+<2>,initial,"""zero"""
+])
+
+dnl The second client should not have the name 'one'.
+AT_CHECK([$PYTHON3 $srcdir/ovsdb-monitor-sort.py < client-2.out | uuidfilt],
+ [0], [dnl
+row,action,name
+<0>,initial,"""two"""
+<1>,initial,"""zero"""
+])
+AT_CLEANUP
--
2.40.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
