diff options
author | Paolo Bonzini <pbonzini@redhat.com> | 2017-09-21 14:32:47 +0200 |
---|---|---|
committer | Paolo Bonzini <pbonzini@redhat.com> | 2017-09-21 23:19:37 +0200 |
commit | 447b0d0b9ee8a0ac216c3186e0f3c427a1001f0c (patch) | |
tree | a12f837b91bfb553f70beef57e6170b0ba386aa3 | |
parent | db81b9953761cac71906728fb3dfefce661ab903 (diff) |
memory: avoid "resurrection" of dead FlatViews
It's possible for address_space_get_flatview() as it currently stands
to cause a use-after-free for the returned FlatView, if the reference
count is incremented after the FlatView has been replaced by a writer:
thread 1 thread 2 RCU thread
-------------------------------------------------------------
rcu_read_lock
read as->current_map
set as->current_map
flatview_unref
'--> call_rcu
flatview_ref
[ref=1]
rcu_read_unlock
flatview_destroy
<badness>
Since FlatViews are not updated very often, we can just detect the
situation using a new atomic op atomic_fetch_inc_nonzero, similar to
Linux's atomic_inc_not_zero, which performs the refcount increment only if
it hasn't already hit zero. This is similar to Linux commit de09a9771a53
("CRED: Fix get_task_cred() and task_state() to not resurrect dead
credentials", 2010-07-29).
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
-rw-r--r-- | docs/devel/atomics.txt | 1 | ||||
-rw-r--r-- | include/qemu/atomic.h | 8 | ||||
-rw-r--r-- | memory.c | 12 |
3 files changed, 17 insertions, 4 deletions
diff --git a/docs/devel/atomics.txt b/docs/devel/atomics.txt index 048e5f23cb..10c5fa37e8 100644 --- a/docs/devel/atomics.txt +++ b/docs/devel/atomics.txt @@ -64,6 +64,7 @@ operations: typeof(*ptr) atomic_fetch_and(ptr, val) typeof(*ptr) atomic_fetch_or(ptr, val) typeof(*ptr) atomic_fetch_xor(ptr, val) + typeof(*ptr) atomic_fetch_inc_nonzero(ptr) typeof(*ptr) atomic_xchg(ptr, val) typeof(*ptr) atomic_cmpxchg(ptr, old, new) diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h index b6b62fb771..d73c9e14d7 100644 --- a/include/qemu/atomic.h +++ b/include/qemu/atomic.h @@ -442,4 +442,12 @@ } while(0) #endif +#define atomic_fetch_inc_nonzero(ptr) ({ \ + typeof_strip_qual(*ptr) _oldn = atomic_read(ptr); \ + while (_oldn && atomic_cmpxchg(ptr, _oldn, _oldn + 1) != _oldn) { \ + _oldn = atomic_read(ptr); \ + } \ + _oldn; \ +}) + #endif /* QEMU_ATOMIC_H */ @@ -294,9 +294,9 @@ static void flatview_destroy(FlatView *view) g_free(view); } -static void flatview_ref(FlatView *view) +static bool flatview_ref(FlatView *view) { - atomic_inc(&view->ref); + return atomic_fetch_inc_nonzero(&view->ref) > 0; } static void flatview_unref(FlatView *view) @@ -773,8 +773,12 @@ static FlatView *address_space_get_flatview(AddressSpace *as) FlatView *view; rcu_read_lock(); - view = atomic_rcu_read(&as->current_map); - flatview_ref(view); + do { + view = atomic_rcu_read(&as->current_map); + /* If somebody has replaced as->current_map concurrently, + * flatview_ref returns false. + */ + } while (!flatview_ref(view)); rcu_read_unlock(); return view; } |