From a521491bc53fa494d08707399cbab764a931d406 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 6 Aug 2018 14:35:27 -0700 Subject: raft: Fix use-after-free error in raft_store_snapshot(). raft_store_snapshot() constructs a new snapshot in a local variable then destroys the current snapshot and replaces it by the new one. Until now, it has not cloned the data in the new snapshot until it did the replacement. This led to the unexpected consequence that, if 'servers' in the old and new snapshots was the same, then it would first be freed and later cloned, which could cause a segfault. Multiple people reported the crash. Gurucharan Shetty provided a reproduction case. Signed-off-by: Ben Pfaff Acked-by: Mark Michelson --- ovsdb/raft.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'ovsdb') diff --git a/ovsdb/raft.c b/ovsdb/raft.c index c0c1e9897..02ba763e5 100644 --- a/ovsdb/raft.c +++ b/ovsdb/raft.c @@ -3838,22 +3838,22 @@ raft_store_snapshot(struct raft *raft, const struct json *new_snapshot_data) } uint64_t new_log_start = raft->last_applied + 1; - const struct raft_entry new_snapshot = { + struct raft_entry new_snapshot = { .term = raft_get_term(raft, new_log_start - 1), - .data = CONST_CAST(struct json *, new_snapshot_data), + .data = json_clone(new_snapshot_data), .eid = *raft_get_eid(raft, new_log_start - 1), - .servers = CONST_CAST(struct json *, - raft_servers_for_index(raft, new_log_start - 1)), + .servers = json_clone(raft_servers_for_index(raft, new_log_start - 1)), }; struct ovsdb_error *error = raft_save_snapshot(raft, new_log_start, &new_snapshot); if (error) { + raft_entry_uninit(&new_snapshot); return error; } raft->log_synced = raft->log_end - 1; raft_entry_uninit(&raft->snap); - raft_entry_clone(&raft->snap, &new_snapshot); + raft->snap = new_snapshot; for (size_t i = 0; i < new_log_start - raft->log_start; i++) { raft_entry_uninit(&raft->entries[i]); } -- cgit v1.2.1