Message ID | 20160909204853.27839-2-joe@ovn.org |
---|---|
State | Superseded |
Headers | show |
On Fri, Sep 09, 2016 at 01:48:51PM -0700, Joe Stringer wrote: > Prior to commit 60e0cd041958 ("ovsdb: Replication usability > improvements"), a changes would never have 'change->new' == NULL. > Apparently this is now possible, which leads to the crash below. > > Valgrind reports: > > ==18725== Process terminating with default action of signal 11 > (SIGSEGV): dumping core > ==18725== Access not within mapped region at address 0x0 > ==18725== at 0x43937E: ovsdb_datum_compare_3way (ovsdb-data.c:1626) > ==18725== by 0x439344: ovsdb_datum_equals (ovsdb-data.c:1616) > ==18725== by 0x4166CC: update_monitor_row_data (monitor.c:310) > ==18725== by 0x414A90: ovsdb_monitor_changes_update (monitor.c:1255) > ==18725== by 0x417009: ovsdb_monitor_change_cb (monitor.c:1339) > ==18725== by 0x41DB52: ovsdb_txn_for_each_change (transaction.c:906) > ==18725== by 0x416CC9: ovsdb_monitor_commit (monitor.c:1553) > ==18725== by 0x41D993: ovsdb_txn_commit_ (transaction.c:868) > ==18725== by 0x41D6F5: ovsdb_txn_commit (transaction.c:893) > ==18725== by 0x418185: process_notification (replication.c:576) > ==18725== by 0x417705: replication_run (replication.c:185) > ==18725== by 0x408240: main_loop (ovsdb-server.c:198) > ==18725== by 0x406432: main (ovsdb-server.c:429) > > Fixes: 60e0cd041958 ("ovsdb: Replication usability improvements") > Signed-off-by: Joe Stringer <joe@ovn.org> > --- > This addresses the crash, but I don't understand the deeper reasoning > why we get into this state in the first place so I welcome any counter > proposals. > > This also affects branch-2.6. Please try this as an alternative: https://patchwork.ozlabs.org/patch/668413/
diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c index 9a6fbf5a5a30..5320c7db0514 100644 --- a/ovsdb/monitor.c +++ b/ovsdb/monitor.c @@ -1252,7 +1252,11 @@ ovsdb_monitor_changes_update(const struct ovsdb_row *old, change->new = clone_monitor_row_data(mt, new); } else { if (new) { - update_monitor_row_data(mt, new, change->new); + if (change->new) { + update_monitor_row_data(mt, new, change->new); + } else { + change->new = clone_monitor_row_data(mt, new); + } } else { free_monitor_row_data(mt, change->new); change->new = NULL;
Prior to commit 60e0cd041958 ("ovsdb: Replication usability improvements"), a changes would never have 'change->new' == NULL. Apparently this is now possible, which leads to the crash below. Valgrind reports: ==18725== Process terminating with default action of signal 11 (SIGSEGV): dumping core ==18725== Access not within mapped region at address 0x0 ==18725== at 0x43937E: ovsdb_datum_compare_3way (ovsdb-data.c:1626) ==18725== by 0x439344: ovsdb_datum_equals (ovsdb-data.c:1616) ==18725== by 0x4166CC: update_monitor_row_data (monitor.c:310) ==18725== by 0x414A90: ovsdb_monitor_changes_update (monitor.c:1255) ==18725== by 0x417009: ovsdb_monitor_change_cb (monitor.c:1339) ==18725== by 0x41DB52: ovsdb_txn_for_each_change (transaction.c:906) ==18725== by 0x416CC9: ovsdb_monitor_commit (monitor.c:1553) ==18725== by 0x41D993: ovsdb_txn_commit_ (transaction.c:868) ==18725== by 0x41D6F5: ovsdb_txn_commit (transaction.c:893) ==18725== by 0x418185: process_notification (replication.c:576) ==18725== by 0x417705: replication_run (replication.c:185) ==18725== by 0x408240: main_loop (ovsdb-server.c:198) ==18725== by 0x406432: main (ovsdb-server.c:429) Fixes: 60e0cd041958 ("ovsdb: Replication usability improvements") Signed-off-by: Joe Stringer <joe@ovn.org> --- This addresses the crash, but I don't understand the deeper reasoning why we get into this state in the first place so I welcome any counter proposals. This also affects branch-2.6. --- ovsdb/monitor.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)