[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 0/6] block/curl: Fix hang and potential crash
From: |
Max Reitz |
Subject: |
[Qemu-devel] [PATCH 0/6] block/curl: Fix hang and potential crash |
Date: |
Tue, 27 Aug 2019 18:34:33 +0200 |
Hi,
As reported in https://bugzilla.redhat.com/show_bug.cgi?id=1740193, our
curl block driver can spontaneously hang. This becomes visible e.g.
when reading compressed qcow2 images:
$ qemu-img convert -p -O raw -n \
https://download.cirros-cloud.net/0.4.0/cirros-0.4.0-x86_64-disk.img \
null-co://
(Hangs at 74.21 %, usually.)
A more direct way is:
$ qemu-img bench -f raw http://download.qemu.org/qemu-4.1.0.tar.xz \
-d 1 -S 524288 -c 2
(Which simply performs two requests, and the second one hangs. You can
use any HTTP resource (probably FTP, too) you’d like that is at least
1 MB in size.)
It turns out that this is because cURL 7.59.0 has added a protective
feature against some misuse we had in our code: curl_multi_add_handle()
must not be called from within a cURL callback, but in some cases we
did. As of 7.59.0, this fails, our new request is not registered and
the I/O request stalls. This is fixed by patch 5.
Patch 6 makes us check for curl_multi_add_handle()’s return code,
because if we had done that before, debugging would have been much
simpler.
On the way to fixing it, I had a look over the whole cURL code and found
a suspicious QLIST_FOREACH_SAFE() loop that actually does not seem very
safe at all. I think this may lead to crashes, although I have never
seen any myself. https://bugzilla.redhat.com/show_bug.cgi?id=1744602#c5
shows one in exactly the function in question, so I think it actually is
a problem.
This is fixed by patch 4, patches 1 through 3 prepare for it.
Max Reitz (6):
curl: Keep pointer to the CURLState in CURLSocket
curl: Keep *socket until the end of curl_sock_cb()
curl: Pass CURLSocket to curl_multi_{do,read}()
curl: Report only ready sockets
curl: Handle success in multi_check_completion
curl: Check curl_multi_add_handle()'s return code
block/curl.c | 129 +++++++++++++++++++++++++--------------------------
1 file changed, 63 insertions(+), 66 deletions(-)
--
2.21.0
- [Qemu-devel] [PATCH 0/6] block/curl: Fix hang and potential crash,
Max Reitz <=
- [Qemu-devel] [PATCH 1/6] curl: Keep pointer to the CURLState in CURLSocket, Max Reitz, 2019/08/27
- [Qemu-devel] [PATCH 2/6] curl: Keep *socket until the end of curl_sock_cb(), Max Reitz, 2019/08/27
- [Qemu-devel] [PATCH 3/6] curl: Pass CURLSocket to curl_multi_{do, read}(), Max Reitz, 2019/08/27
- [Qemu-devel] [PATCH 4/6] curl: Report only ready sockets, Max Reitz, 2019/08/27
- [Qemu-devel] [PATCH 5/6] curl: Handle success in multi_check_completion, Max Reitz, 2019/08/27
- [Qemu-devel] [PATCH 6/6] curl: Check curl_multi_add_handle()'s return code, Max Reitz, 2019/08/27