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>

diff --git a/docs/devel/atomics.txt b/docs/devel/atomics.txt
index 3ef5d85b1be2100bcc8c7d5af2e0bf8b236d018e..3fbaf52140d17a3888a25e31927fdcadb33a3667 100644 (file)
--- 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 b6b62fb7710979297596624278816e9af0d9f40c..d73c9e14d7f30c9f62246ae82366f896dbfe9de8 100644 (file)
--- 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 de57a16ece631d76a1f927b689d87a4391bca296..41e2e67301b43effa7d54eec899e299451f2ee48 100644 (file)
--- 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;
 }