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>
This commit is contained in:
Paolo Bonzini 2017-09-21 14:32:47 +02:00
parent db81b99537
commit 447b0d0b9e
3 changed files with 17 additions and 4 deletions

View file

@ -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)

View file

@ -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 */

View file

@ -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;
}