Re: [PATCH] migration: free 'saddr' since be no longer used

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

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<zhouzongmin@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


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|