Skip to content

Commit

Permalink
raft: Fix use-after-free error in raft_store_snapshot().
Browse files Browse the repository at this point in the history
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 <blp@ovn.org>
Acked-by: Mark Michelson <mmichels@redhat.com>
  • Loading branch information
blp committed Aug 7, 2018
1 parent 4a3cf22 commit 40bacfd
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions ovsdb/raft.c
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Expand Down

0 comments on commit 40bacfd

Please sign in to comment.