On Wed, Nov 15, 2023 at 09:49:09AM +0000, Daniel P. Berrangé wrote:
> On Wed, Nov 15, 2023 at 11:27:39AM +0800, Zongmin Zhou wrote:
> > Since socket_parse() will allocate memory for 'saddr',
> > and its value will pass to 'addr' that allocated
> > by migrate_uri_parse(),so free 'saddr' to avoid memory leak.
> >
> > Fixes: 72a8192e225c ("migration: convert migration 'uri' into
> > 'MigrateAddress'")
> > Signed-off-by: Zongmin Zhou<zhouzong...@kylinos.cn>
> > ---
> > migration/migration.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 28a34c9068..30ed4bf6b6 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -493,6 +493,7 @@ bool migrate_uri_parse(const char *uri,
> > MigrationChannel **channel,
> > }
> > addr->u.socket.type = saddr->type;
> > addr->u.socket.u = saddr->u;
>
> 'saddr->u' is a union embedded in SocketAddress, containing:
>
> union { /* union tag is @type */
> InetSocketAddressWrapper inet;
> UnixSocketAddressWrapper q_unix;
> VsockSocketAddressWrapper vsock;
> StringWrapper fd;
> } u;
>
> THis assignment is *shallow* copying the contents of the union.
>
> All the type specifics structs that are members of this union
> containing allocated strings, and with this shallow copy, we
> are stealing the pointers to these allocated strings
>
>
> > + qapi_free_SocketAddress(saddr);
>
> This meanwhle is doing a *deep* free of the contents of the
> SocketAddress, which includes all the pointers we just stole.
>
> IOW, unless I'm mistaken somehow, this is going to cause a
> double-free
Right. I think what we need is a g_free(saddr), with a comment explaining?
Or, is there better way to do that? Something like a QAPI_CLONE() but not
exactly: we already have the object allocated. We want to deep copy it to
the current object only on the fields but not the object itself.
--
Peter Xu
