From 7de5d186bd03511cd5fa4dcd32876efb034f1dd0 Mon Sep 17 00:00:00 2001

From: David Gibson <dgibson@redhat.com>

Date: Thu, 16 Nov 2017 03:07:13 +0100

Subject: [PATCH 09/30] memory: avoid "resurrection" of dead FlatViews

RH-Author: David Gibson <dgibson@redhat.com>

Message-id: <20171116030732.8560-4-dgibson@redhat.com>

Patchwork-id: 77691

O-Subject: [PATCH 03/22] memory: avoid "resurrection" of dead FlatViews

Bugzilla: 1481593

RH-Acked-by: Thomas Huth <thuth@redhat.com>

RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>

RH-Acked-by: Eduardo Habkost <ehabkost@redhat.com>

RH-Acked-by: Laurent Vivier <lvivier@redhat.com>

From: Paolo Bonzini <pbonzini@redhat.com>

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)

Signed-off-by: David Gibson <dgibson@redhat.com>

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

---

 docs/devel/atomics.txt |  1 +

 include/qemu/atomic.h  |  8 ++++++++

 memory.c               | 12 ++++++++----

 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/docs/devel/atomics.txt b/docs/devel/atomics.txt

index 048e5f2..10c5fa3 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 b6b62fb..d73c9e1 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 c0adc35..ca160de 100644

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

@@ -772,8 +772,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;

 }

-- 

1.8.3.1