| Message ID | 20210412170255.231406-1-sgarzare@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cutils: fix memory leak in get_relocated_path() | expand |
Is this fix aiming at 6.0 release? On 4/12/21 7:02 PM, Stefano Garzarella wrote: > get_relocated_path() allocates a GString object and returns the > character data (C string) to the caller without freeing the memory > allocated for that object as reported by valgrind: > > 24 bytes in 1 blocks are definitely lost in loss record 2,805 of 6,532 > at 0x4839809: malloc (vg_replace_malloc.c:307) > by 0x55AABB8: g_malloc (in /usr/lib64/libglib-2.0.so.0.6600.8) > by 0x55C2481: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.6600.8) > by 0x55C4827: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.6600.8) > by 0x55C4CEA: g_string_new (in /usr/lib64/libglib-2.0.so.0.6600.8) > by 0x906314: get_relocated_path (cutils.c:1036) > by 0x6E1F77: qemu_read_default_config_file (vl.c:2122) > by 0x6E1F77: qemu_init (vl.c:2687) > by 0x3E3AF8: main (main.c:49) > > Let's use g_string_free(gstring, false) to free only the GString object > and transfer the ownership of the character data to the caller. > > Fixes: f4f5ed2cbd ("cutils: introduce get_relocated_path") > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > --- > util/cutils.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/util/cutils.c b/util/cutils.c > index ee908486da..c9b91e7535 100644 > --- a/util/cutils.c > +++ b/util/cutils.c > @@ -1055,5 +1055,5 @@ char *get_relocated_path(const char *dir) > assert(G_IS_DIR_SEPARATOR(dir[-1])); > g_string_append(result, dir - 1); > } > - return result->str; > + return g_string_free(result, false); > } >
On Tue, Apr 13, 2021 at 12:59:36PM +0200, Philippe Mathieu-Daudé wrote: >Is this fix aiming at 6.0 release? The leak is minimal, but the fix is very simple. So, I think it can go if someone has a pull request to send with other patches, but I'm not sure with which tree. Thanks, Stefano > >On 4/12/21 7:02 PM, Stefano Garzarella wrote: >> get_relocated_path() allocates a GString object and returns the >> character data (C string) to the caller without freeing the memory >> allocated for that object as reported by valgrind: >> >> 24 bytes in 1 blocks are definitely lost in loss record 2,805 of 6,532 >> at 0x4839809: malloc (vg_replace_malloc.c:307) >> by 0x55AABB8: g_malloc (in /usr/lib64/libglib-2.0.so.0.6600.8) >> by 0x55C2481: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.6600.8) >> by 0x55C4827: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.6600.8) >> by 0x55C4CEA: g_string_new (in /usr/lib64/libglib-2.0.so.0.6600.8) >> by 0x906314: get_relocated_path (cutils.c:1036) >> by 0x6E1F77: qemu_read_default_config_file (vl.c:2122) >> by 0x6E1F77: qemu_init (vl.c:2687) >> by 0x3E3AF8: main (main.c:49) >> >> Let's use g_string_free(gstring, false) to free only the GString object >> and transfer the ownership of the character data to the caller. >> >> Fixes: f4f5ed2cbd ("cutils: introduce get_relocated_path") >> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >> --- >> util/cutils.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/util/cutils.c b/util/cutils.c >> index ee908486da..c9b91e7535 100644 >> --- a/util/cutils.c >> +++ b/util/cutils.c >> @@ -1055,5 +1055,5 @@ char *get_relocated_path(const char *dir) >> assert(G_IS_DIR_SEPARATOR(dir[-1])); >> g_string_append(result, dir - 1); >> } >> - return result->str; >> + return g_string_free(result, false); >> } >> >
On 4/13/21 1:47 PM, Stefano Garzarella wrote: > On Tue, Apr 13, 2021 at 12:59:36PM +0200, Philippe Mathieu-Daudé wrote: >> Is this fix aiming at 6.0 release? > > The leak is minimal, but the fix is very simple. > So, I think it can go if someone has a pull request to send with other > patches, but I'm not sure with which tree. I'd say Paolo... Cc'ing Daniel/Marc-André who have a good GLib understanding. > > Thanks, > Stefano > >> >> On 4/12/21 7:02 PM, Stefano Garzarella wrote: >>> get_relocated_path() allocates a GString object and returns the >>> character data (C string) to the caller without freeing the memory >>> allocated for that object as reported by valgrind: >>> >>> 24 bytes in 1 blocks are definitely lost in loss record 2,805 of 6,532 >>> at 0x4839809: malloc (vg_replace_malloc.c:307) >>> by 0x55AABB8: g_malloc (in /usr/lib64/libglib-2.0.so.0.6600.8) >>> by 0x55C2481: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.6600.8) >>> by 0x55C4827: g_string_sized_new (in >>> /usr/lib64/libglib-2.0.so.0.6600.8) >>> by 0x55C4CEA: g_string_new (in /usr/lib64/libglib-2.0.so.0.6600.8) >>> by 0x906314: get_relocated_path (cutils.c:1036) >>> by 0x6E1F77: qemu_read_default_config_file (vl.c:2122) >>> by 0x6E1F77: qemu_init (vl.c:2687) >>> by 0x3E3AF8: main (main.c:49) >>> >>> Let's use g_string_free(gstring, false) to free only the GString object >>> and transfer the ownership of the character data to the caller. >>> >>> Fixes: f4f5ed2cbd ("cutils: introduce get_relocated_path") >>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >>> --- >>> util/cutils.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/util/cutils.c b/util/cutils.c >>> index ee908486da..c9b91e7535 100644 >>> --- a/util/cutils.c >>> +++ b/util/cutils.c >>> @@ -1055,5 +1055,5 @@ char *get_relocated_path(const char *dir) >>> assert(G_IS_DIR_SEPARATOR(dir[-1])); >>> g_string_append(result, dir - 1); >>> } >>> - return result->str; >>> + return g_string_free(result, false); >>> } >>> >> >
On Mon, Apr 12, 2021 at 9:06 PM Stefano Garzarella <sgarzare@redhat.com> wrote: > get_relocated_path() allocates a GString object and returns the > character data (C string) to the caller without freeing the memory > allocated for that object as reported by valgrind: > > 24 bytes in 1 blocks are definitely lost in loss record 2,805 of 6,532 > at 0x4839809: malloc (vg_replace_malloc.c:307) > by 0x55AABB8: g_malloc (in /usr/lib64/libglib-2.0.so.0.6600.8) > by 0x55C2481: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.6600.8) > by 0x55C4827: g_string_sized_new (in > /usr/lib64/libglib-2.0.so.0.6600.8) > by 0x55C4CEA: g_string_new (in /usr/lib64/libglib-2.0.so.0.6600.8) > by 0x906314: get_relocated_path (cutils.c:1036) > by 0x6E1F77: qemu_read_default_config_file (vl.c:2122) > by 0x6E1F77: qemu_init (vl.c:2687) > by 0x3E3AF8: main (main.c:49) > > Let's use g_string_free(gstring, false) to free only the GString object > and transfer the ownership of the character data to the caller. > > Fixes: f4f5ed2cbd ("cutils: introduce get_relocated_path") > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- > util/cutils.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/util/cutils.c b/util/cutils.c > index ee908486da..c9b91e7535 100644 > --- a/util/cutils.c > +++ b/util/cutils.c > @@ -1055,5 +1055,5 @@ char *get_relocated_path(const char *dir) > assert(G_IS_DIR_SEPARATOR(dir[-1])); > g_string_append(result, dir - 1); > } > - return result->str; > + return g_string_free(result, false); > } > -- > 2.30.2 > > >
Ping :-) Should I resend for 6.1? Thanks Stefano On Mon, Apr 12, 2021 at 07:02:55PM +0200, Stefano Garzarella wrote: >get_relocated_path() allocates a GString object and returns the >character data (C string) to the caller without freeing the memory >allocated for that object as reported by valgrind: > > 24 bytes in 1 blocks are definitely lost in loss record 2,805 of 6,532 > at 0x4839809: malloc (vg_replace_malloc.c:307) > by 0x55AABB8: g_malloc (in /usr/lib64/libglib-2.0.so.0.6600.8) > by 0x55C2481: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.6600.8) > by 0x55C4827: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.6600.8) > by 0x55C4CEA: g_string_new (in /usr/lib64/libglib-2.0.so.0.6600.8) > by 0x906314: get_relocated_path (cutils.c:1036) > by 0x6E1F77: qemu_read_default_config_file (vl.c:2122) > by 0x6E1F77: qemu_init (vl.c:2687) > by 0x3E3AF8: main (main.c:49) > >Let's use g_string_free(gstring, false) to free only the GString object >and transfer the ownership of the character data to the caller. > >Fixes: f4f5ed2cbd ("cutils: introduce get_relocated_path") >Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> >--- > util/cutils.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/util/cutils.c b/util/cutils.c >index ee908486da..c9b91e7535 100644 >--- a/util/cutils.c >+++ b/util/cutils.c >@@ -1055,5 +1055,5 @@ char *get_relocated_path(const char *dir) > assert(G_IS_DIR_SEPARATOR(dir[-1])); > g_string_append(result, dir - 1); > } >- return result->str; >+ return g_string_free(result, false); > } >-- >2.30.2 > >
Stefano Garzarella <sgarzare@redhat.com> writes: > Ping :-) > > Should I resend for 6.1? I'm cc'ing qemu-trivial. For good measure: Reviewed-by: Markus Armbruster <armbru@redhat.com>
Le 11/05/2021 à 07:38, Markus Armbruster a écrit : > Stefano Garzarella <sgarzare@redhat.com> writes: > >> Ping :-) >> >> Should I resend for 6.1? > > I'm cc'ing qemu-trivial. > > For good measure: > Reviewed-by: Markus Armbruster <armbru@redhat.com> > > Applied to my trivial-patches branch. Thanks, Laurent
diff --git a/util/cutils.c b/util/cutils.c index ee908486da..c9b91e7535 100644 --- a/util/cutils.c +++ b/util/cutils.c @@ -1055,5 +1055,5 @@ char *get_relocated_path(const char *dir) assert(G_IS_DIR_SEPARATOR(dir[-1])); g_string_append(result, dir - 1); } - return result->str; + return g_string_free(result, false); }
get_relocated_path() allocates a GString object and returns the character data (C string) to the caller without freeing the memory allocated for that object as reported by valgrind: 24 bytes in 1 blocks are definitely lost in loss record 2,805 of 6,532 at 0x4839809: malloc (vg_replace_malloc.c:307) by 0x55AABB8: g_malloc (in /usr/lib64/libglib-2.0.so.0.6600.8) by 0x55C2481: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.6600.8) by 0x55C4827: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.6600.8) by 0x55C4CEA: g_string_new (in /usr/lib64/libglib-2.0.so.0.6600.8) by 0x906314: get_relocated_path (cutils.c:1036) by 0x6E1F77: qemu_read_default_config_file (vl.c:2122) by 0x6E1F77: qemu_init (vl.c:2687) by 0x3E3AF8: main (main.c:49) Let's use g_string_free(gstring, false) to free only the GString object and transfer the ownership of the character data to the caller. Fixes: f4f5ed2cbd ("cutils: introduce get_relocated_path") Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> --- util/cutils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)