summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaolo Bonzini <pbonzini@redhat.com>2017-09-21 14:32:47 +0200
committerMichael Roth <mdroth@linux.vnet.ibm.com>2017-12-04 22:03:33 -0600
commit3568e119400aeffa98e32187c40d2802b7d4f574 (patch)
tree2e692048f75899643c170bdea692ab34b779d929
parentd0136db81292ccac38a74b4ef2d3c3853b7c1355 (diff)
downloadqemu-3568e119400aeffa98e32187c40d2802b7d4f574.tar.gz
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> (cherry picked from commit 447b0d0b9ee8a0ac216c3186e0f3c427a1001f0c) Conflicts: docs/devel/atomics.txt * drop documentation ref to atomic_fetch_xor * prereq for 166206845f Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
-rw-r--r--docs/devel/atomics.txt1
-rw-r--r--include/qemu/atomic.h8
-rw-r--r--memory.c12
3 files changed, 17 insertions, 4 deletions
diff --git a/docs/devel/atomics.txt b/docs/devel/atomics.txt
index 3ef5d85b1b..3fbaf52140 100644
--- a/docs/devel/atomics.txt
+++ b/docs/devel/atomics.txt
@@ -63,6 +63,7 @@ operations:
typeof(*ptr) atomic_fetch_sub(ptr, val)
typeof(*ptr) atomic_fetch_and(ptr, val)
typeof(*ptr) atomic_fetch_or(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 */
diff --git a/memory.c b/memory.c
index de57a16ece..41e2e67301 100644
--- a/memory.c
+++ b/memory.c
@@ -300,9 +300,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)
@@ -792,8 +792,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;
}