cryptospore / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone
958e1b
From 4283e6a300b8b05f8fb32dc27dfe0bd774ef16db Mon Sep 17 00:00:00 2001
958e1b
From: Fam Zheng <famz@redhat.com>
958e1b
Date: Fri, 30 May 2014 03:46:55 +0200
958e1b
Subject: [PATCH 08/13] aio: Fix use-after-free in cancellation path
958e1b
958e1b
RH-Author: Fam Zheng <famz@redhat.com>
958e1b
Message-id: <1401421615-17300-1-git-send-email-famz@redhat.com>
958e1b
Patchwork-id: 59078
958e1b
O-Subject: [RHEL-7 qemu-kvm PATCH] aio: Fix use-after-free in cancellation path
958e1b
Bugzilla: 1095877
958e1b
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
958e1b
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
958e1b
RH-Acked-by: Kevin Wolf <kwolf@redhat.com>
958e1b
958e1b
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1095877
958e1b
Brew:     https://brewweb.devel.redhat.com/taskinfo?taskID=7519118 (RHEL)
958e1b
          https://brewweb.devel.redhat.com/taskinfo?taskID=7519129 (RHEV)
958e1b
958e1b
The current flow of canceling a thread from THREAD_ACTIVE state is:
958e1b
958e1b
  1) Caller wants to cancel a request, so it calls thread_pool_cancel.
958e1b
958e1b
  2) thread_pool_cancel waits on the conditional variable
958e1b
     elem->check_cancel.
958e1b
958e1b
  3) The worker thread changes state to THREAD_DONE once the task is
958e1b
     done, and notifies elem->check_cancel to allow thread_pool_cancel
958e1b
     to continue execution, and signals the notifier (pool->notifier) to
958e1b
     allow callback function to be called later. But because of the
958e1b
     global mutex, the notifier won't get processed until step 4) and 5)
958e1b
     are done.
958e1b
958e1b
  4) thread_pool_cancel continues, leaving the notifier signaled, it
958e1b
     just returns to caller.
958e1b
958e1b
  5) Caller thinks the request is already canceled successfully, so it
958e1b
     releases any related data, such as freeing elem->common.opaque.
958e1b
958e1b
  6) In the next main loop iteration, the notifier handler,
958e1b
     event_notifier_ready, is called. It finds the canceled thread in
958e1b
     THREAD_DONE state, so calls elem->common.cb, with an (likely)
958e1b
     dangling opaque pointer. This is a use-after-free.
958e1b
958e1b
Fix it by calling event_notifier_ready before leaving
958e1b
thread_pool_cancel.
958e1b
958e1b
Test case update: This change will let cancel complete earlier than
958e1b
test-thread-pool.c expects, so update the code to check this case: if
958e1b
it's already done, done_cb sets .aiocb to NULL, skip calling
958e1b
bdrv_aio_cancel on them.
958e1b
958e1b
Reported-by: Ulrich Obergfell <uobergfe@redhat.com>
958e1b
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
958e1b
Signed-off-by: Fam Zheng <famz@redhat.com>
958e1b
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
958e1b
(cherry picked from commit 271c0f68b4eae72691721243a1c37f46a3232d61)
958e1b
Signed-off-by: Fam Zheng <famz@redhat.com>
958e1b
---
958e1b
 tests/test-thread-pool.c | 2 +-
958e1b
 thread-pool.c            | 1 +
958e1b
 2 files changed, 2 insertions(+), 1 deletion(-)
958e1b
958e1b
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
958e1b
---
958e1b
 tests/test-thread-pool.c |    2 +-
958e1b
 thread-pool.c            |    1 +
958e1b
 2 files changed, 2 insertions(+), 1 deletions(-)
958e1b
958e1b
diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
958e1b
index b62338f..1be970d 100644
958e1b
--- a/tests/test-thread-pool.c
958e1b
+++ b/tests/test-thread-pool.c
958e1b
@@ -181,7 +181,7 @@ static void test_cancel(void)
958e1b
 
958e1b
     /* Canceling the others will be a blocking operation.  */
958e1b
     for (i = 0; i < 100; i++) {
958e1b
-        if (data[i].n != 3) {
958e1b
+        if (data[i].aiocb && data[i].n != 3) {
958e1b
             bdrv_aio_cancel(data[i].aiocb);
958e1b
         }
958e1b
     }
958e1b
diff --git a/thread-pool.c b/thread-pool.c
958e1b
index 0ebd4c2..fc6a33b 100644
958e1b
--- a/thread-pool.c
958e1b
+++ b/thread-pool.c
958e1b
@@ -229,6 +229,7 @@ static void thread_pool_cancel(BlockDriverAIOCB *acb)
958e1b
         pool->pending_cancellations--;
958e1b
     }
958e1b
     qemu_mutex_unlock(&pool->lock);
958e1b
+    event_notifier_ready(&pool->notifier);
958e1b
 }
958e1b
 
958e1b
 static const AIOCBInfo thread_pool_aiocb_info = {
958e1b
-- 
958e1b
1.7.1
958e1b