Re: [PATCH] qemu-pr-helper: do not free reply when multipath_process_cmd() fails

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Oct 19, 2026 at 04:57:10PM -0400, Stefan Hajnoczi wrote:
> On Mon, Oct 19, 2026 at 11:19:29AM -0500, Eric Blake wrote:
> > On Mon, Oct 19, 2026 at 11:33:53AM -0400, Stefan Hajnoczi wrote:
> > > Kevin Wolf pointed out that the multipath_process_cmd() documentation
> > > only defines the value of reply on success. Don't touch it when the
> > > function returns -1.
> > > 
> > > The libmpathcmd implementation as of c78cf64a9e8e does not set *reply to
> > > NULL when its internal mpath_send_cmd() call fails, so I think Kevin's
> > > interpretation is correct.
> > > 
> > > As it stands, there is no observable bug in the QEMU code though because
> > > reply is initialized to NULL before calling multipath_process_cmd().
> > > This patch is just a cleanup.
> > > 
> > > Cc: Benjamin Marzinski <bmarzins@redhat.com>
> > > Cc: kwolf@redhat.com
> > > Cc: eblake@redhat.com
> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > ---
> > > Eric pointed out that g_autofree could eliminate the manual free(3)
> > > calls. In this case we cannot use it since g_autofree calls glib's
> > > g_free() rather than the standard library's free(3).
> > 
> > I know glib encourages g_free() in case malloc was interposed, but
> > aren't we guaranteed that we are not exploiting glib's malloc
> > overrides, and therefore g_free() calls into free(3)?  But I'm fine
> > with your decision to not use g_autofree here.
> 
> g_mem_set_vtable() is deprecated and a nop. Interposing no longer works.
> g_mem_is_system_malloc() always returns TRUE.
> 
> If we take advantage of glib internals, then you can call g_free() on
> malloc() memory AFAIK. But I don't think the API documentation
> guarantees this will always be the case and I'm not sure it's worth
> baking in this assumption.

Glib has public guaranteed this for years now, since I personally added
this guarantee to their API docs :-)

  "Since GLib 2.46, g_malloc() is hardcoded to always use the system
   malloc implementation."

   https://docs.gtk.org/glib/memory.html

QEMU has equally relied on this guarantee for years.

The only things we have to be wary of are other non-glib based 3rd
party libraries QEMU uses that may allow malloc overrides.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|