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 <[email protected]>

diff --git a/docs/devel/atomics.txt b/docs/devel/atomics.txt
index 048e5f23cbdecad4450ded819534b7580cf1a010..10c5fa37e8cc85061adce8b19bb5988955e7f14e 100644 (file)
--- 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 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 2b90117c6015255e0d29af8624c39d6fe7a31be0..51f54ab43013077a711e3ee6852cc37427933aa5 100644 (file)
--- a/memory.c
+++ b/memory.c
@@ -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;
 }