|
|
4a2fec |
From 7de5d186bd03511cd5fa4dcd32876efb034f1dd0 Mon Sep 17 00:00:00 2001
|
|
|
4a2fec |
From: David Gibson <dgibson@redhat.com>
|
|
|
4a2fec |
Date: Thu, 16 Nov 2017 03:07:13 +0100
|
|
|
4a2fec |
Subject: [PATCH 09/30] memory: avoid "resurrection" of dead FlatViews
|
|
|
4a2fec |
|
|
|
4a2fec |
RH-Author: David Gibson <dgibson@redhat.com>
|
|
|
4a2fec |
Message-id: <20171116030732.8560-4-dgibson@redhat.com>
|
|
|
4a2fec |
Patchwork-id: 77691
|
|
|
4a2fec |
O-Subject: [PATCH 03/22] memory: avoid "resurrection" of dead FlatViews
|
|
|
4a2fec |
Bugzilla: 1481593
|
|
|
4a2fec |
RH-Acked-by: Thomas Huth <thuth@redhat.com>
|
|
|
4a2fec |
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
|
|
|
4a2fec |
RH-Acked-by: Eduardo Habkost <ehabkost@redhat.com>
|
|
|
4a2fec |
RH-Acked-by: Laurent Vivier <lvivier@redhat.com>
|
|
|
4a2fec |
|
|
|
4a2fec |
From: Paolo Bonzini <pbonzini@redhat.com>
|
|
|
4a2fec |
|
|
|
4a2fec |
It's possible for address_space_get_flatview() as it currently stands
|
|
|
4a2fec |
to cause a use-after-free for the returned FlatView, if the reference
|
|
|
4a2fec |
count is incremented after the FlatView has been replaced by a writer:
|
|
|
4a2fec |
|
|
|
4a2fec |
thread 1 thread 2 RCU thread
|
|
|
4a2fec |
-------------------------------------------------------------
|
|
|
4a2fec |
rcu_read_lock
|
|
|
4a2fec |
read as->current_map
|
|
|
4a2fec |
set as->current_map
|
|
|
4a2fec |
flatview_unref
|
|
|
4a2fec |
'--> call_rcu
|
|
|
4a2fec |
flatview_ref
|
|
|
4a2fec |
[ref=1]
|
|
|
4a2fec |
rcu_read_unlock
|
|
|
4a2fec |
flatview_destroy
|
|
|
4a2fec |
<badness>
|
|
|
4a2fec |
|
|
|
4a2fec |
Since FlatViews are not updated very often, we can just detect the
|
|
|
4a2fec |
situation using a new atomic op atomic_fetch_inc_nonzero, similar to
|
|
|
4a2fec |
Linux's atomic_inc_not_zero, which performs the refcount increment only if
|
|
|
4a2fec |
it hasn't already hit zero. This is similar to Linux commit de09a9771a53
|
|
|
4a2fec |
("CRED: Fix get_task_cred() and task_state() to not resurrect dead
|
|
|
4a2fec |
credentials", 2010-07-29).
|
|
|
4a2fec |
|
|
|
4a2fec |
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
|
|
|
4a2fec |
(cherry picked from commit 447b0d0b9ee8a0ac216c3186e0f3c427a1001f0c)
|
|
|
4a2fec |
|
|
|
4a2fec |
Signed-off-by: David Gibson <dgibson@redhat.com>
|
|
|
4a2fec |
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
|
|
|
4a2fec |
---
|
|
|
4a2fec |
docs/devel/atomics.txt | 1 +
|
|
|
4a2fec |
include/qemu/atomic.h | 8 ++++++++
|
|
|
4a2fec |
memory.c | 12 ++++++++----
|
|
|
4a2fec |
3 files changed, 17 insertions(+), 4 deletions(-)
|
|
|
4a2fec |
|
|
|
4a2fec |
diff --git a/docs/devel/atomics.txt b/docs/devel/atomics.txt
|
|
|
4a2fec |
index 048e5f2..10c5fa3 100644
|
|
|
4a2fec |
--- a/docs/devel/atomics.txt
|
|
|
4a2fec |
+++ b/docs/devel/atomics.txt
|
|
|
4a2fec |
@@ -64,6 +64,7 @@ operations:
|
|
|
4a2fec |
typeof(*ptr) atomic_fetch_and(ptr, val)
|
|
|
4a2fec |
typeof(*ptr) atomic_fetch_or(ptr, val)
|
|
|
4a2fec |
typeof(*ptr) atomic_fetch_xor(ptr, val)
|
|
|
4a2fec |
+ typeof(*ptr) atomic_fetch_inc_nonzero(ptr)
|
|
|
4a2fec |
typeof(*ptr) atomic_xchg(ptr, val)
|
|
|
4a2fec |
typeof(*ptr) atomic_cmpxchg(ptr, old, new)
|
|
|
4a2fec |
|
|
|
4a2fec |
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
|
|
|
4a2fec |
index b6b62fb..d73c9e1 100644
|
|
|
4a2fec |
--- a/include/qemu/atomic.h
|
|
|
4a2fec |
+++ b/include/qemu/atomic.h
|
|
|
4a2fec |
@@ -442,4 +442,12 @@
|
|
|
4a2fec |
} while(0)
|
|
|
4a2fec |
#endif
|
|
|
4a2fec |
|
|
|
4a2fec |
+#define atomic_fetch_inc_nonzero(ptr) ({ \
|
|
|
4a2fec |
+ typeof_strip_qual(*ptr) _oldn = atomic_read(ptr); \
|
|
|
4a2fec |
+ while (_oldn && atomic_cmpxchg(ptr, _oldn, _oldn + 1) != _oldn) { \
|
|
|
4a2fec |
+ _oldn = atomic_read(ptr); \
|
|
|
4a2fec |
+ } \
|
|
|
4a2fec |
+ _oldn; \
|
|
|
4a2fec |
+})
|
|
|
4a2fec |
+
|
|
|
4a2fec |
#endif /* QEMU_ATOMIC_H */
|
|
|
4a2fec |
diff --git a/memory.c b/memory.c
|
|
|
4a2fec |
index c0adc35..ca160de 100644
|
|
|
4a2fec |
--- a/memory.c
|
|
|
4a2fec |
+++ b/memory.c
|
|
|
4a2fec |
@@ -294,9 +294,9 @@ static void flatview_destroy(FlatView *view)
|
|
|
4a2fec |
g_free(view);
|
|
|
4a2fec |
}
|
|
|
4a2fec |
|
|
|
4a2fec |
-static void flatview_ref(FlatView *view)
|
|
|
4a2fec |
+static bool flatview_ref(FlatView *view)
|
|
|
4a2fec |
{
|
|
|
4a2fec |
- atomic_inc(&view->ref);
|
|
|
4a2fec |
+ return atomic_fetch_inc_nonzero(&view->ref) > 0;
|
|
|
4a2fec |
}
|
|
|
4a2fec |
|
|
|
4a2fec |
static void flatview_unref(FlatView *view)
|
|
|
4a2fec |
@@ -772,8 +772,12 @@ static FlatView *address_space_get_flatview(AddressSpace *as)
|
|
|
4a2fec |
FlatView *view;
|
|
|
4a2fec |
|
|
|
4a2fec |
rcu_read_lock();
|
|
|
4a2fec |
- view = atomic_rcu_read(&as->current_map);
|
|
|
4a2fec |
- flatview_ref(view);
|
|
|
4a2fec |
+ do {
|
|
|
4a2fec |
+ view = atomic_rcu_read(&as->current_map);
|
|
|
4a2fec |
+ /* If somebody has replaced as->current_map concurrently,
|
|
|
4a2fec |
+ * flatview_ref returns false.
|
|
|
4a2fec |
+ */
|
|
|
4a2fec |
+ } while (!flatview_ref(view));
|
|
|
4a2fec |
rcu_read_unlock();
|
|
|
4a2fec |
return view;
|
|
|
4a2fec |
}
|
|
|
4a2fec |
--
|
|
|
4a2fec |
1.8.3.1
|
|
|
4a2fec |
|