[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-stable] [PATCH v2 0/7] block/curl: Fix hang and potential cras
From: |
Max Reitz |
Subject: |
Re: [Qemu-stable] [PATCH v2 0/7] block/curl: Fix hang and potential crash |
Date: |
Fri, 13 Sep 2019 13:48:31 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
On 10.09.19 14:41, Max Reitz wrote:
> 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 6.
>
> Patch 7 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 5, patches 1, 2, and 4 prepare for it.
>
> (Patch 3 is kind of a misc patch that should ensure that we always end
> up calling curl_multi_check_completion() whenever a request might have
> been completed.)
Thanks for the review, applied to my block branch:
https://git.xanclic.moe/XanClic/qemu/commits/branch/block
Max
signature.asc
Description: OpenPGP digital signature