debian/patches/old/0001-aio-Fix-use-after-free-in-cancellation-path.patch

   1 From 271c0f68b4eae72691721243a1c37f46a3232d61 Mon Sep 17 00:00:00 2001
   2 From: Fam Zheng <famz@redhat.com>
   3 Date: Wed, 21 May 2014 10:42:13 +0800
   4 Subject: [PATCH] aio: Fix use-after-free in cancellation path
   5
   6 The current flow of canceling a thread from THREAD_ACTIVE state is:
   7
   8   1) Caller wants to cancel a request, so it calls thread_pool_cancel.
   9
  10   2) thread_pool_cancel waits on the conditional variable
  11      elem->check_cancel.
  12
  13   3) The worker thread changes state to THREAD_DONE once the task is
  14      done, and notifies elem->check_cancel to allow thread_pool_cancel
  15      to continue execution, and signals the notifier (pool->notifier) to
  16      allow callback function to be called later. But because of the
  17      global mutex, the notifier won't get processed until step 4) and 5)
  18      are done.
  19
  20   4) thread_pool_cancel continues, leaving the notifier signaled, it
  21      just returns to caller.
  22
  23   5) Caller thinks the request is already canceled successfully, so it
  24      releases any related data, such as freeing elem->common.opaque.
  25
  26   6) In the next main loop iteration, the notifier handler,
  27      event_notifier_ready, is called. It finds the canceled thread in
  28      THREAD_DONE state, so calls elem->common.cb, with an (likely)
  29      dangling opaque pointer. This is a use-after-free.
  30
  31 Fix it by calling event_notifier_ready before leaving
  32 thread_pool_cancel.
  33
  34 Test case update: This change will let cancel complete earlier than
  35 test-thread-pool.c expects, so update the code to check this case: if
  36 it's already done, done_cb sets .aiocb to NULL, skip calling
  37 bdrv_aio_cancel on them.
  38
  39 Reported-by: Ulrich Obergfell <uobergfe@redhat.com>
  40 Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
  41 Signed-off-by: Fam Zheng <famz@redhat.com>
  42 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
  43 ---
  44  tests/test-thread-pool.c |    2 +-
  45  thread-pool.c            |    1 +
  46  2 files changed, 2 insertions(+), 1 deletion(-)
  47
  48 diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
  49 index c1f8e13..aa156bc 100644
  50 --- a/tests/test-thread-pool.c
  51 +++ b/tests/test-thread-pool.c
  52 @@ -180,7 +180,7 @@ static void test_cancel(void)
  53
  54      /* Canceling the others will be a blocking operation.  */
  55      for (i = 0; i < 100; i++) {
  56 -        if (data[i].n != 3) {
  57 +        if (data[i].aiocb && data[i].n != 3) {
  58              bdrv_aio_cancel(data[i].aiocb);
  59          }
  60      }
  61 diff --git a/thread-pool.c b/thread-pool.c
  62 index fbdd3ff..dfb699d 100644
  63 --- a/thread-pool.c
  64 +++ b/thread-pool.c
  65 @@ -224,6 +224,7 @@ static void thread_pool_cancel(BlockDriverAIOCB *acb)
  66          pool->pending_cancellations--;
  67      }
  68      qemu_mutex_unlock(&pool->lock);
  69 +    event_notifier_ready(&pool->notifier);
  70  }
  71
  72  static const AIOCBInfo thread_pool_aiocb_info = {
  73 --
  74 1.7.10.4
  75