A condition on a column that is not monitored adds that column to the
monitor via ovsdb_monitor_add_column(). Commit 469e98e16db1 destroys the
initial change set in this case, but the change set that tracks new
changes keeps the old number of columns and is still handed out by
ovsdb_monitor_track_new_change_set() to the session with the new
condition. Rows added to it later have no datum for the new column,
and evaluating the condition on them reads past the end of the row data:
ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 4 at 0x506000006e40 thread T0
0 ovsdb_clause_evaluate ovsdb/condition.c:329
1 ovsdb_condition_match_any_clause ovsdb/condition.c:442
2 ovsdb_condition_empty_or_match_any ovsdb/condition.h:84
3 ovsdb_monitor_row_update_type_condition ovsdb/monitor.c:898
4 ovsdb_monitor_compose_row_update2 ovsdb/monitor.c:1064
5 ovsdb_monitor_compose_update ovsdb/monitor.c:1180
6 ovsdb_monitor_get_update ovsdb/monitor.c:1286
7 ovsdb_jsonrpc_monitor_compose_update ovsdb/jsonrpc-server.c:1832
8 ovsdb_jsonrpc_monitor_flush_all ovsdb/jsonrpc-server.c:1909
Between transactions, the change set that tracks new changes has no
rows, since it stops being tracked as new as soon as a transaction adds
rows to it. Update its number of columns when a column is added. Also
move its starting transaction to the last one, since changes to the new
column made by earlier transactions were not tracked and the set must
not be reused by monitor_cond_since requests that need them.
Fixes: e0f42d4a6548 ("monitor: Fix crash when monitor condition adds
new columns.")
Assisted-by: claude-opus-5-5, Claude Code
Signed-off-by: Alejandro Pacheco <[email protected]>
---
ovsdb/monitor.c | 20 ++++++
tests/ovsdb-monitor.at | 141 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 161 insertions(+)
diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c
index 4998687c1..7e94e6bad 100644
--- a/ovsdb/monitor.c
+++ b/ovsdb/monitor.c
@@ -519,6 +519,26 @@ ovsdb_monitor_add_column(struct ovsdb_monitor *dbmon,
mt->n_monitored_columns++;
}
+ mcs = dbmon->new_change_set;
+ if (mcs) {
+ /* The change set that tracks new changes is handed out to sessions
+ * whose conditions may use the new column. Between transactions it
+ * doesn't have any rows, since it stops being tracked as new as soon
+ * as a transaction adds rows to it, so it only needs to cover the new
+ * column. Changes to the new column made by earlier transactions
+ * were not tracked, so it can't be reused for them. */
+ struct ovsdb_monitor_change_set_for_table *mcst;
+
+ LIST_FOR_EACH (mcst, list_in_change_set,
+ &mcs->change_set_for_tables) {
+ if (mcst->mt == mt) {
+ ovs_assert(hmap_is_empty(&mcst->rows));
+ mcst->n_columns = mt->n_columns;
+ }
+ }
+ mcs->prev_txn = *ovsdb_monitor_get_last_txnid(dbmon);
+ }
+
return NULL;
}
diff --git a/tests/ovsdb-monitor.at b/tests/ovsdb-monitor.at
index 2e03ef015..c58fb5fcf 100644
--- a/tests/ovsdb-monitor.at
+++ b/tests/ovsdb-monitor.at
@@ -1072,3 +1072,144 @@ row,action,name
<1>,initial,"""zero"""
])
AT_CLEANUP
+
+AT_SETUP([monitor-cond updates with condition on non-monitored column])
+AT_KEYWORDS([ovsdb server monitor monitor-cond positive update 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])
+
+AT_CHECK([ovsdb-client transact unix:socket '[["ordinals",
+ {"op": "insert",
+ "table": "ordinals",
+ "row": {"number": 0, "name": "zero"}}]]'], [0], [ignore], [ignore])
+
+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])
+
+dnl Both clients receive updates from the same change set.
+AT_CHECK([ovsdb-client transact unix:socket '[["ordinals",
+ {"op": "insert",
+ "table": "ordinals",
+ "row": {"number": 1, "name": "one"}},
+ {"op": "insert",
+ "table": "ordinals",
+ "row": {"number": 2, "name": "two"}}]]'], [0], [ignore], [ignore])
+OVS_WAIT_UNTIL([grep -q 'two' client-1.out && grep -q 'two' 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,"""zero"""
+
+row,action,name
+<1>,insert,"""one"""
+<2>,insert,"""two"""
+])
+
+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,"""zero"""
+
+row,action,name
+<1>,insert,"""two"""
+])
+AT_CLEANUP
+
+AT_SETUP([monitor-cond-since with condition on non-monitored column])
+AT_KEYWORDS([ovsdb server monitor monitor-cond-since positive non-monitored])
+
+ordinal_schema > schema
+AT_CHECK([ovsdb-tool create-cluster db schema unix:db.raft], [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])
+
+AT_CHECK([ovsdb-client transact unix:socket '[["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"}},
+ {"op": "insert",
+ "table": "ordinals",
+ "row": {"number": 3, "name": "three"}}]]'], [0], [ignore], [ignore])
+
+dnl A small transaction, so that the history keeps it.
+AT_CHECK([ovsdb-client transact unix:socket '[["ordinals",
+ {"op": "insert",
+ "table": "ordinals",
+ "row": {"number": 10, "name": "ten"}}]]'], [0], [ignore], [ignore])
+
+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-since --format=csv unix:socket \
+ ordinals '[[true]]' ordinals ["name"] \
+ > client-1.out 2> client-1.err], [0], [ignore], [ignore])
+OVS_WAIT_UNTIL([grep -q 'initial' client-1.out])
+last_id=$(sed -n 's/.*last_id: \([[0-9a-f-]]*\).*/\1/p' client-1.out)
+
+dnl Change only the column 'number', which no client monitors yet.
+AT_CHECK([ovsdb-client transact unix:socket '[["ordinals",
+ {"op": "update",
+ "table": "ordinals",
+ "where": [["number", "==", 1]],
+ "row": {"number": 5}}]]'], [0], [ignore], [ignore])
+
+dnl Start a second client from the first transaction with a condition on
+dnl column 'number'. It should get the change made by the second one.
+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-since --format=csv unix:socket \
+ ordinals $last_id '[[["number", "==", 5]]]' ordinals ["name"] \
+ > client-2.out 2> client-2.err], [0], [ignore], [ignore])
+OVS_WAIT_UNTIL([grep -q 'found' client-2.out])
+
+OVSDB_SERVER_SHUTDOWN
+OVS_WAIT_UNTIL([test ! -e ovsdb-server.pid && \
+ test ! -e client-1.pid && test ! -e client-2.pid])
+
+AT_CHECK([$PYTHON3 $srcdir/ovsdb-monitor-sort.py < client-2.out | uuidfilt],
+ [0], [dnl
+found: true, last_id: <0>
+row,action,name
+<1>,insert,"""one"""
+])
+AT_CLEANUP
--
2.53.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
