Skip to content
  • Fam Zheng's avatar
    aio: Fix use-after-free in cancellation path · 271c0f68
    Fam Zheng authored
    
    
    The current flow of canceling a thread from THREAD_ACTIVE state is:
    
      1) Caller wants to cancel a request, so it calls thread_pool_cancel.
    
      2) thread_pool_cancel waits on the conditional variable
         elem->check_cancel.
    
      3) The worker thread changes state to THREAD_DONE once the task is
         done, and notifies elem->check_cancel to allow thread_pool_cancel
         to continue execution, and signals the notifier (pool->notifier) to
         allow callback function to be called later. But because of the
         global mutex, the notifier won't get processed until step 4) and 5)
         are done.
    
      4) thread_pool_cancel continues, leaving the notifier signaled, it
         just returns to caller.
    
      5) Caller thinks the request is already canceled successfully, so it
         releases any related data, such as freeing elem->common.opaque.
    
      6) In the next main loop iteration, the notifier handler,
         event_notifier_ready, is called. It finds the canceled thread in
         THREAD_DONE state, so calls elem->common.cb, with an (likely)
         dangling opaque pointer. This is a use-after-free.
    
    Fix it by calling event_notifier_ready before leaving
    thread_pool_cancel.
    
    Test case update: This change will let cancel complete earlier than
    test-thread-pool.c expects, so update the code to check this case: if
    it's already done, done_cb sets .aiocb to NULL, skip calling
    bdrv_aio_cancel on them.
    
    Reported-by: default avatarUlrich Obergfell <uobergfe@redhat.com>
    Suggested-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
    Signed-off-by: default avatarFam Zheng <famz@redhat.com>
    Signed-off-by: default avatarStefan Hajnoczi <stefanha@redhat.com>
    271c0f68