From: Han Zhou <hzh...@ebay.com> When a client uses monitor-cond-since with a non-zero last-id but the server is not in cluster mode for the DB being monitored, it leads to segmentation fault because the txn_history list is not initialized in this case.
Program terminated with signal SIGSEGV, Segmentation fault. 1536 struct ovsdb_txn *txn = h_node->txn; (gdb) bt 0 ovsdb_monitor_get_changes_after (txn_uuid=txn_uuid@entry=0x7ffe8605b7e0, dbmon=0x17c1b40, p_mcs=p_mcs@entry=0x17c4900) at ovsdb/monitor.c:1536 1 0x000000000040da2d in ovsdb_jsonrpc_monitor_create (request_id=0x1804630, version=<optimized out>, params=0x17ad330, db=0x18015b0, s=<optimized out>) at ovsdb/jsonrpc-server.c:1469 2 ovsdb_jsonrpc_session_got_request (request=0x17ad520, s=<optimized out>) at ovsdb/jsonrpc-server.c:1002 3 ovsdb_jsonrpc_session_run (s=<optimized out>) at ovsdb/jsonrpc-server.c:556 ... Although it doesn't happen in normal use cases, no one can prevent a client to send this on purpose or in a corner case when a client firstly connected to a clustered DB but later the server restarted with a non-clustered DB. This patch fixes it by always initialize the txn_history list to avoid the undefined behavior in this case. It adds a test case to cover it, too. Fixes: 695e815 ("ovsdb-server: Transaction history tracking.") Reported-by: Aliasgar Ginwala <aginw...@ebay.com> Signed-off-by: Han Zhou <hzh...@ebay.com> --- ovsdb/ovsdb-server.c | 5 ++--- ovsdb/transaction.c | 4 ++-- ovsdb/transaction.h | 2 +- tests/ovsdb-monitor.at | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 6 deletions(-) diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c index 9dc1d57..9827320 100644 --- a/ovsdb/ovsdb-server.c +++ b/ovsdb/ovsdb-server.c @@ -664,9 +664,8 @@ open_db(struct server_config *config, const char *filename) /* Enable txn history for clustered mode. It is not enabled for other mode * for now, since txn id is available for clustered mode only. */ - if (ovsdb_storage_is_clustered(storage)) { - ovsdb_txn_history_init(db->db); - } + ovsdb_txn_history_init(db->db, ovsdb_storage_is_clustered(storage)); + read_db(config, db); error = (db->db->name[0] == '_' diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c index 67ea771..866fabe 100644 --- a/ovsdb/transaction.c +++ b/ovsdb/transaction.c @@ -1416,9 +1416,9 @@ ovsdb_txn_history_run(struct ovsdb *db) } void -ovsdb_txn_history_init(struct ovsdb *db) +ovsdb_txn_history_init(struct ovsdb *db, bool need_txn_history) { - db->need_txn_history = true; + db->need_txn_history = need_txn_history; db->n_txn_history = 0; ovs_list_init(&db->txn_history); } diff --git a/ovsdb/transaction.h b/ovsdb/transaction.h index c21871a..ea6b53d 100644 --- a/ovsdb/transaction.h +++ b/ovsdb/transaction.h @@ -63,7 +63,7 @@ void ovsdb_txn_for_each_change(const struct ovsdb_txn *, void ovsdb_txn_add_comment(struct ovsdb_txn *, const char *); const char *ovsdb_txn_get_comment(const struct ovsdb_txn *); void ovsdb_txn_history_run(struct ovsdb *); -void ovsdb_txn_history_init(struct ovsdb *); +void ovsdb_txn_history_init(struct ovsdb *, bool need_txn_history); void ovsdb_txn_history_destroy(struct ovsdb *); #endif /* ovsdb/transaction.h */ diff --git a/tests/ovsdb-monitor.at b/tests/ovsdb-monitor.at index 84aa208..eb43709 100644 --- a/tests/ovsdb-monitor.at +++ b/tests/ovsdb-monitor.at @@ -956,3 +956,58 @@ row,action,name,number,_version ]], [ignore]) AT_CLEANUP + +# Test monitor-cond-since with non-cluster mode server with non-zero last_id +AT_SETUP([monitor-cond-since non-cluster non-zero last_id]) +AT_KEYWORDS([ovsdb server monitor monitor-cond-since negative]) +ordinal_schema > schema +AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore]) +AT_CAPTURE_FILE([ovsdb-server-log]) +AT_CHECK([ovsdb-server --detach --no-chdir --pidfile --remote=punix:socket --log-file="`pwd`"/ovsdb-server-log db >/dev/null 2>&1]) +on_exit 'kill `cat ovsdb-server.pid`' +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 + +# A non-zero uuid +last_id=11111111-1111-1111-1111-111111111111 +AT_CHECK([ovsdb-client -vjsonrpc --pidfile --detach --no-chdir -d json monitor-cond-since --format=csv unix:socket ordinals $last_id '[[["name","==","one"],["name","==","ten"]]]' ordinals > output], + [0], [ignore], [ignore]) +on_exit 'kill `cat ovsdb-client.pid`' +for txn in m4_foreach([txn], [[[["ordinals", + {"op": "insert", + "table": "ordinals", + "row": {"number": 10, "name": "ten"}}, + {"op": "insert", + "table": "ordinals", + "row": {"number": 11, "name": "eleven"}}]]]], ['txn' ]); do + AT_CHECK([ovsdb-client transact unix:socket "$txn"], [0], + [ignore], [ignore], [kill `cat server-pid client-pid`]) +done +AT_CHECK([ovsdb-client transact unix:socket '[["ordinals"]]'], [0], + [ignore], [ignore]) +AT_CHECK([ovs-appctl -t ovsdb-server -e exit], [0], [ignore], [ignore]) +OVS_WAIT_UNTIL([test ! -e ovsdb-server.pid && test ! -e ovsdb-client.pid]) + +# Transaction shouldn't be found, and last_id returned should always +# be the same (all zero uuid) +AT_CHECK([$PYTHON $srcdir/ovsdb-monitor-sort.py < output | uuidfilt], [0], + [[found: false, last_id: <0> +row,action,name,number,_version +<1>,initial,"""one""",1,"[""uuid"",""<2>""]" + +last_id: <0> +row,action,name,number,_version +<3>,insert,"""ten""",10,"[""uuid"",""<4>""]" +]], [ignore]) +AT_CLEANUP + -- 2.1.0 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev