From: "trieu2.huynh" <trieu2.huynh@lge.com>

The function curl_header_cb uses g_autofree with g_strstrip(g_strndup(...)).
However, g_strstrip may return a pointer that is an offset from the
original allocated memory, causing g_autofree to attempt to free
an invalid pointer or leak the original.

Separate the allocation and the stripping to ensure the original
pointer is correctly tracked and freed.

Resolves: CID 1645633

Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
---
 block/curl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/curl.c b/block/curl.c
index 66aecfb20e..5b66c80704 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -208,7 +208,8 @@ static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
 {
     BDRVCURLState *s = opaque;
     size_t realsize = size * nmemb;
-    g_autofree char *header = g_strstrip(g_strndup(ptr, realsize));
+    g_autofree char *header = g_strndup(ptr, realsize);
+    g_strstrip(header);
     char *val = strchr(header, ':');
 
     if (!val) {
-- 
2.43.0

On Thu, 12 Mar 2026 at 08:53, Trieu Huynh <vikingtc4@gmail.com> wrote:
>
> From: "trieu2.huynh" <trieu2.huynh@lge.com>
>
> The function curl_header_cb uses g_autofree with g_strstrip(g_strndup(...)).
> However, g_strstrip may return a pointer that is an offset from the
> original allocated memory, causing g_autofree to attempt to free
> an invalid pointer or leak the original.

I don't believe this is correct. g_strstrip() will
always return the string argument it is passed. (The glib
documentation for g_strstrip() doesn't say so explicitly, but
it is a macro for g_strchomp(g_strchug(string)), and both
those functions say that they return the input argmuent.)

> Separate the allocation and the stripping to ensure the original
> pointer is correctly tracked and freed.
>
> Resolves: CID 1645633
>
> Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
> ---
>  block/curl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/block/curl.c b/block/curl.c
> index 66aecfb20e..5b66c80704 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -208,7 +208,8 @@ static size_t curl_header_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
>  {
>      BDRVCURLState *s = opaque;
>      size_t realsize = size * nmemb;
> -    g_autofree char *header = g_strstrip(g_strndup(ptr, realsize));
> +    g_autofree char *header = g_strndup(ptr, realsize);
> +    g_strstrip(header);

Being able to rewrite the code like this confirms that we
don't actually have a leak -- we are still relying here on
g_strstrip(X) == X, just in a different way.

>      char *val = strchr(header, ':');
>
>      if (!val) {

This looks like a Coverity false positive to me, so I've marked it
that way in the Coverity Scan UI.

thanks
-- PMM

ack.

BRs,


Vào Thứ 5, 12 thg 3, 2026 vào lúc 16:34 Peter Maydell <
peter.maydell@linaro.org> đã viết:

> On Thu, 12 Mar 2026 at 08:53, Trieu Huynh <vikingtc4@gmail.com> wrote:
> >
> > From: "trieu2.huynh" <trieu2.huynh@lge.com>
> >
> > The function curl_header_cb uses g_autofree with
> g_strstrip(g_strndup(...)).
> > However, g_strstrip may return a pointer that is an offset from the
> > original allocated memory, causing g_autofree to attempt to free
> > an invalid pointer or leak the original.
>
> I don't believe this is correct. g_strstrip() will
> always return the string argument it is passed. (The glib
> documentation for g_strstrip() doesn't say so explicitly, but
> it is a macro for g_strchomp(g_strchug(string)), and both
> those functions say that they return the input argmuent.)
>
> > Separate the allocation and the stripping to ensure the original
> > pointer is correctly tracked and freed.
> >
> > Resolves: CID 1645633
> >
> > Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
> > ---
> >  block/curl.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/curl.c b/block/curl.c
> > index 66aecfb20e..5b66c80704 100644
> > --- a/block/curl.c
> > +++ b/block/curl.c
> > @@ -208,7 +208,8 @@ static size_t curl_header_cb(void *ptr, size_t size,
> size_t nmemb, void *opaque)
> >  {
> >      BDRVCURLState *s = opaque;
> >      size_t realsize = size * nmemb;
> > -    g_autofree char *header = g_strstrip(g_strndup(ptr, realsize));
> > +    g_autofree char *header = g_strndup(ptr, realsize);
> > +    g_strstrip(header);
>
> Being able to rewrite the code like this confirms that we
> don't actually have a leak -- we are still relying here on
> g_strstrip(X) == X, just in a different way.
>
> >      char *val = strchr(header, ':');
> >
> >      if (!val) {
>
> This looks like a Coverity false positive to me, so I've marked it
> that way in the Coverity Scan UI.
>
> thanks
> -- PMM
>