[PATCH] block/curl: fix curl internal handles handling

Michael Tokarev posted 1 patch 2 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250824001144.2001882-1-mjt@tls.msk.ru
Maintainers: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>
block/curl.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
[PATCH] block/curl: fix curl internal handles handling
Posted by Michael Tokarev 2 months, 3 weeks ago
block/curl.c uses CURLMOPT_SOCKETFUNCTION to register a socket callback.
According to the documentation, this callback is called not just with
application-created sockets but also with internal curl sockets, - and
for such sockets, user data pointer is not set by the application, so
the result qemu crashing.

Pass BDRVCURLState directly to the callback function as user pointer,
instead of relying on CURLINFO_PRIVATE.

This problem started happening with update of libcurl from 8.9 to 8.10 --
apparently with this change curl started using private handles more.

(CURLINFO_PRIVATE is used in one more place, in curl_multi_check_completion() -
it might need a similar fix too)

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3081
Cc: qemu-stable@qemu.org
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
---
 block/curl.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index 5467678024..00b949ea45 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -162,13 +162,9 @@ static int curl_timer_cb(CURLM *multi, long timeout_ms, void *opaque)
 static int curl_sock_cb(CURL *curl, curl_socket_t fd, int action,
                         void *userp, void *sp)
 {
-    BDRVCURLState *s;
-    CURLState *state = NULL;
+    BDRVCURLState *s = userp;
     CURLSocket *socket;
 
-    curl_easy_getinfo(curl, CURLINFO_PRIVATE, (char **)&state);
-    s = state->s;
-
     socket = g_hash_table_lookup(s->sockets, GINT_TO_POINTER(fd));
     if (!socket) {
         socket = g_new0(CURLSocket, 1);
@@ -605,6 +601,7 @@ static void curl_attach_aio_context(BlockDriverState *bs,
     assert(!s->multi);
     s->multi = curl_multi_init();
     s->aio_context = new_context;
+    curl_multi_setopt(s->multi, CURLMOPT_SOCKETDATA, s);
     curl_multi_setopt(s->multi, CURLMOPT_SOCKETFUNCTION, curl_sock_cb);
     curl_multi_setopt(s->multi, CURLMOPT_TIMERDATA, s);
     curl_multi_setopt(s->multi, CURLMOPT_TIMERFUNCTION, curl_timer_cb);
-- 
2.47.2
Re: [PATCH] block/curl: fix curl internal handles handling
Posted by Michael Tokarev 2 months, 2 weeks ago
On 24.08.2025 03:11, Michael Tokarev wrote:
> block/curl.c uses CURLMOPT_SOCKETFUNCTION to register a socket callback.
> According to the documentation, this callback is called not just with
> application-created sockets but also with internal curl sockets, - and
> for such sockets, user data pointer is not set by the application, so
> the result qemu crashing.
> 
> Pass BDRVCURLState directly to the callback function as user pointer,
> instead of relying on CURLINFO_PRIVATE.
> 
> This problem started happening with update of libcurl from 8.9 to 8.10 --
> apparently with this change curl started using private handles more.
> 
> (CURLINFO_PRIVATE is used in one more place, in curl_multi_check_completion() -
> it might need a similar fix too)
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3081

Kevin, qemu-block@ -- does it look okay if I'll push this one through
trivial-patches tree?  It's not exactly trivial (but simple enough),
but I'd rather fix this issue, here and for debian.

Or are you going to send a pullreq for block?

Thanks,

/mjt
Re: [PATCH] block/curl: fix curl internal handles handling
Posted by Michael Tokarev 2 months, 2 weeks ago
Ping#2?  We've curl access broken entirely with current versions of
curl..

With no replies, I'm going to merge it through trivial-patches in a day
or two.

Thanks,

/mjt

On 28.08.2025 12:05, Michael Tokarev wrote:
> On 24.08.2025 03:11, Michael Tokarev wrote:
>> block/curl.c uses CURLMOPT_SOCKETFUNCTION to register a socket callback.
>> According to the documentation, this callback is called not just with
>> application-created sockets but also with internal curl sockets, - and
>> for such sockets, user data pointer is not set by the application, so
>> the result qemu crashing.
>>
>> Pass BDRVCURLState directly to the callback function as user pointer,
>> instead of relying on CURLINFO_PRIVATE.
>>
>> This problem started happening with update of libcurl from 8.9 to 8.10 --
>> apparently with this change curl started using private handles more.
>>
>> (CURLINFO_PRIVATE is used in one more place, in 
>> curl_multi_check_completion() -
>> it might need a similar fix too)
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3081
> 
> Kevin, qemu-block@ -- does it look okay if I'll push this one through
> trivial-patches tree?  It's not exactly trivial (but simple enough),
> but I'd rather fix this issue, here and for debian.
> 
> Or are you going to send a pullreq for block?



Re: [PATCH] block/curl: fix curl internal handles handling
Posted by Daniel P. Berrangé 2 months, 2 weeks ago
On Sun, Aug 24, 2025 at 03:11:42AM +0300, Michael Tokarev wrote:
> block/curl.c uses CURLMOPT_SOCKETFUNCTION to register a socket callback.
> According to the documentation, this callback is called not just with
> application-created sockets but also with internal curl sockets, - and
> for such sockets, user data pointer is not set by the application, so
> the result qemu crashing.
> 
> Pass BDRVCURLState directly to the callback function as user pointer,
> instead of relying on CURLINFO_PRIVATE.
> 
> This problem started happening with update of libcurl from 8.9 to 8.10 --
> apparently with this change curl started using private handles more.
> 
> (CURLINFO_PRIVATE is used in one more place, in curl_multi_check_completion() -
> it might need a similar fix too)
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3081
> Cc: qemu-stable@qemu.org
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
>  block/curl.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|