From e8ba6a37c3ab919f93c82639ea646d34c0bdc23a Mon Sep 17 00:00:00 2001

From: Paolo Bonzini <pbonzini@redhat.com>

Date: Wed, 17 May 2017 13:09:17 +0200

Subject: [PATCH 11/27] curl: avoid recursive locking of BDRVCURLState mutex

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

Message-id: <20170517130921.27402-4-pbonzini@redhat.com>

Patchwork-id: 75261

O-Subject: [RHEL7.4 qemu-kvm PATCH v2 3/7] curl: avoid recursive locking of BDRVCURLState mutex

Bugzilla: 1437393

RH-Acked-by: Max Reitz <mreitz@redhat.com>

RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

RH-Acked-by: Jeffrey Cody <jcody@redhat.com>

The curl driver has a ugly hack where, if it cannot find an empty CURLState,

it just uses aio_poll to wait for one to be empty.  This is probably

buggy when used together with dataplane, and the simplest way to fix it

is to use coroutines instead.

A more immediate effect of the bug however is that it can cause a

recursive call to curl_readv_bh_cb and recursively taking the

BDRVCURLState mutex.  This causes a deadlock.

The fix is to unlock the mutex around aio_poll, but for cleanliness we

should also take the mutex around all calls to curl_init_state, even if

reaching the unlock/lock pair is impossible.  The same is true for

curl_clean_state.

Reported-by: Kun Wei <kuwei@redhat.com>

Tested-by: Richard W.M. Jones <rjones@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Jeff Cody <jcody@redhat.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Message-id: 20170515100059.15795-4-pbonzini@redhat.com

Cc: qemu-stable@nongnu.org

Cc: Jeff Cody <jcody@redhat.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Jeff Cody <jcody@redhat.com>

(cherry picked from commit 456af346297ebef86aa097b3609534d34f3d2f75)

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

---

 block/curl.c | 14 +++++++++++++-

 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/block/curl.c b/block/curl.c

index 94a1e16..7001476 100644

--- a/block/curl.c

+++ b/block/curl.c

@@ -281,6 +281,7 @@ read_end:

     return size * nmemb;

 }

+/* Called with s->mutex held.  */

 static int curl_find_buf(BDRVCURLState *s, size_t start, size_t len,

                          CURLAIOCB *acb)

 {

@@ -453,6 +454,7 @@ static void curl_multi_timeout_do(void *arg)

 #endif

 }

+/* Called with s->mutex held.  */

 static CURLState *curl_init_state(BlockDriverState *bs, BDRVCURLState *s)

 {

     CURLState *state = NULL;

@@ -471,7 +473,9 @@ static CURLState *curl_init_state(BlockDriverState *bs, BDRVCURLState *s)

             break;

         }

         if (!state) {

+            qemu_mutex_unlock(&s->mutex);

             aio_poll(bdrv_get_aio_context(bs), true);

+            qemu_mutex_lock(&s->mutex);

         }

     } while(!state);

@@ -534,6 +538,7 @@ static CURLState *curl_init_state(BlockDriverState *bs, BDRVCURLState *s)

     return state;

 }

+/* Called with s->mutex held.  */

 static void curl_clean_state(CURLState *s)

 {

     int j;

@@ -565,6 +570,7 @@ static void curl_detach_aio_context(BlockDriverState *bs)

     BDRVCURLState *s = bs->opaque;

     int i;

+    qemu_mutex_lock(&s->mutex);

     for (i = 0; i < CURL_NUM_STATES; i++) {

         if (s->states[i].in_use) {

             curl_clean_state(&s->states[i]);

@@ -580,6 +586,7 @@ static void curl_detach_aio_context(BlockDriverState *bs)

         curl_multi_cleanup(s->multi);

         s->multi = NULL;

     }

+    qemu_mutex_unlock(&s->mutex);

     timer_del(&s->timer);

 }

@@ -677,6 +684,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,

         return -EROFS;

     }

+    qemu_mutex_init(&s->mutex);

     opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);

     qemu_opts_absorb_qdict(opts, options, &local_err);

     if (local_err) {

@@ -747,7 +755,9 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,

     DPRINTF("CURL: Opening %s\n", file);

     s->aio_context = bdrv_get_aio_context(bs);

     s->url = g_strdup(file);

+    qemu_mutex_lock(&s->mutex);

     state = curl_init_state(bs, s);

+    qemu_mutex_unlock(&s->mutex);

     if (!state)

         goto out_noclean;

@@ -791,11 +801,12 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,

     }

     DPRINTF("CURL: Size = %zd\n", s->len);

+    qemu_mutex_lock(&s->mutex);

     curl_clean_state(state);

+    qemu_mutex_unlock(&s->mutex);

     curl_easy_cleanup(state->curl);

     state->curl = NULL;

-    qemu_mutex_init(&s->mutex);

     curl_attach_aio_context(bs, bdrv_get_aio_context(bs));

     qemu_opts_del(opts);

@@ -806,6 +817,7 @@ out:

     curl_easy_cleanup(state->curl);

     state->curl = NULL;

 out_noclean:

+    qemu_mutex_destroy(&s->mutex);

     g_free(s->cookie);

     g_free(s->url);

     qemu_opts_del(opts);

-- 

1.8.3.1