On 24/3/26 04:25, 赵国汗 wrote:
> Should we directly call curl_close() here instead? Otherwise
> factor a common curl_cleanup() out and reuse?
Thanks for the suggestion.
I don't think curl_close() can be used directly from out_noclean, since
out_noclean is reached from partially initialized states.
curl_close() assumes a successfully opened instance and tears down the
aio/curl state via curl_detach_aio_context(), while out_noclean is used
before curl_attach_aio_context() and, in some cases, before s->sockets is
initialized.
So I think reusing curl_close() here would conflate the open-failure and
normal close paths.
If we want to deduplicate this later, factoring out only the common field
cleanup into a small helper should be safer.
Yes, this is what I meant.
Regards,
Phil.
*主 题：*Re: [PATCH] block/curl: free s->password in cleanup paths
*日 期：*2026年03月20日17:24
*发件人：*Philippe Mathieu-Daudé
*收件人：*zhaoguohan_salmon,Kevin Wolf,Hanna Reitz,Philippe Mathieu-Daudé
*抄送人：*qemu-block,qemu-devel
Hi, On 20/3/26 07:30, [email protected] wrote: > From: GuoHan
Zhao <[email protected]> > > When password-secret is used,
curl_open() resolves it with > qcrypto_secret_lookup_as_utf8() and
stores the returned buffer in > s->password. > > Unlike s-
>proxypassword, s->password is not freed either in the open > failure
path or in curl_close(), so the resolved secret leaks once it > has been
allocated. > > Free s->password in both cleanup paths. > > Signed-off-
by: GuoHan Zhao <[email protected]> > --- > block/curl.c | 2 ++ > 1
file changed, 2 insertions(+) > > diff --git a/block/curl.c b/block/
curl.c > index 66aecfb20ec6..419df78258bc 100644 > --- a/block/curl.c >
+++ b/block/curl.c > @@ -903,6 +903,7 @@ out_noclean: > g_free(s-
>cookie); > g_free(s->url); > g_free(s->username); > + g_free(s-
>password); > g_free(s->proxyusername); > g_free(s->proxypassword); >
if (s->sockets) { Should we directly call curl_close() here instead?
Otherwise factor a common curl_cleanup() out and reuse? > @@ -1014,6
+1015,7 @@ static void curl_close(BlockDriverState *bs) > g_free(s-
>cookie); > g_free(s->url); > g_free(s->username); > + g_free(s-
>password); > g_free(s->proxyusername); > g_free(s->proxypassword); > }
</[email protected]></[email protected]>
---
