[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-stable] [PATCH 30/79] curl: avoid recursive locking of BDRVCURLSta
From: |
Michael Roth |
Subject: |
[Qemu-stable] [PATCH 30/79] curl: avoid recursive locking of BDRVCURLState mutex |
Date: |
Mon, 28 Aug 2017 19:14:05 -0500 |
From: Paolo Bonzini <address@hidden>
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 <address@hidden>
Tested-by: Richard W.M. Jones <address@hidden>
Reviewed-by: Max Reitz <address@hidden>
Reviewed-by: Jeff Cody <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
Message-id: address@hidden
Cc: address@hidden
Cc: Jeff Cody <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
Signed-off-by: Jeff Cody <address@hidden>
(cherry picked from commit 456af346297ebef86aa097b3609534d34f3d2f75)
Signed-off-by: Michael Roth <address@hidden>
---
block/curl.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/block/curl.c b/block/curl.c
index 59e69c6..dddf5ef 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);
--
2.7.4
- [Qemu-stable] [PATCH 17/79] s390x: Drop useless casts, (continued)
- [Qemu-stable] [PATCH 17/79] s390x: Drop useless casts, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 22/79] qemu-img: wait for convert coroutines to complete, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 21/79] aio: add missing aio_notify() to aio_enable_external(), Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 20/79] hw/virtio: fix vhost user fails to startup when MQ, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 23/79] block/vhdx: Make vhdx_create() always set errp, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 25/79] blockdev: use drained_begin/end for qmp_block_resize, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 26/79] target/xtensa: fix mapping direction in read/write simcalls, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 27/79] target/xtensa: fix return value of read/write simcalls, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 28/79] curl: strengthen assertion in curl_clean_state, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 24/79] block: Add errp to b{lk, drv}_truncate(), Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 30/79] curl: avoid recursive locking of BDRVCURLState mutex,
Michael Roth <=
- [Qemu-stable] [PATCH 33/79] virtio: allow broken device to notify guest, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 35/79] e1000e: Fix ICR "Other" causes clear logic, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 29/79] curl: never invoke callbacks with s->mutex held, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 18/79] qobject: Use simpler QDict/QList scalar insertion macros, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 03/79] qemu-img/convert: Always set ret < 0 on error, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 32/79] vvfat: fix qemu-img map and qemu-img convert, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 36/79] s390x/css: catch section mismatch on load, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 02/79] dirty-bitmap: Report BlockDirtyInfo.count in bytes, as documented, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 39/79] blkdebug: Refactor error injection, Michael Roth, 2017/08/28
- [Qemu-stable] [PATCH 43/79] tests: Add coverage for recent block geometry fixes, Michael Roth, 2017/08/28