[ovs-dev] [PATCH] raft: Fix use-after-free error in raft_store_snapshot().
Ben Pfaff
blp at ovn.org
Tue Aug 7 19:25:48 UTC 2018
Thanks, applied to master, branch-2.10, branch-2.9.
On Tue, Aug 07, 2018 at 02:26:11PM -0400, Mark Michelson wrote:
> Looks good to me.
>
> Acked-by: Mark Michelson <mmichels at redhat.com>
>
> On 08/06/2018 05:35 PM, Ben Pfaff wrote:
> >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 at ovn.org>
> >---
> > ovsdb/raft.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> >diff --git a/ovsdb/raft.c b/ovsdb/raft.c
> >index c0c1e98977b9..02ba763e5fc4 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]);
> > }
> >
>
More information about the dev
mailing list