ovsdb monitor: Fix crash when using non-zero last-id with standalone DB.

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 <aginwala@ebay.com>
Signed-off-by: Han Zhou <hzhou8@ebay.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>

4 files changed, 60 insertions, 6 deletions

diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 9dc1d57af..9827320ec 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 67ea771d6..866fabe8d 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 c21871a45..ea6b53d3c 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 84aa20826..eb437092c 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
+