Re: [PATCH] 'channel' and 'addr' in qmp_migrate() are not auto-freed. migrate_uri_parse() allocates memory which is returned to 'channel', which is leaked because there is no code for freeing 'channel' or 'addr'. So, free addr and channel to avoid memory leak. 'addr' does shallow copying of channel->addr, hence free 'channel' itself and deep free contents of 'addr'

[Het Gala]

On 28/11/23 12:46 pm, Markus Armbruster wrote:

> Your commit message is all in one line.  You need to format it like
>
>      migration: Plug memory leak
>
>     'channel' and 'addr' in qmp_migrate() are not auto-freed.
>     migrate_uri_parse() allocates memory which is returned to 'channel',
>     which is leaked because there is no code for freeing 'channel' or
>     'addr'.  So, free addr and channel to avoid memory leak.  'addr'
>     does shallow copying of channel->addr, hence free 'channel' itself
>     and deep free contents of 'addr'.
>
> Het Gala <het.gala@nutanix.com> writes:
>     
    Yeah, I made the changes in v2 patchset.

> > ---
> >  migration/migration.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 28a34c9068..29efb51b62 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -2004,6 +2004,8 @@ void qmp_migrate(const char *uri, bool has_channels,
> >                            MIGRATION_STATUS_FAILED);
> >          block_cleanup_parameters();
> >      }
> > +    g_free(channel);
> > +    qapi_free_MigrationAddress(addr);
> >  
> >      if (local_err) {
> >          if (!resume_requested) {
> >       
> 2. hmp_migrate()
>
>    hmp_migrate() allocates @channel with migrate_uri_parse(), adds it to
>    list @caps, passes @caps to qmp_migrate(), then frees @caps with
>    qapi_free_MigrationChannelList().
>

Markus, sorry if I was not able to put point clearly, what I meant is that the local 'channel' variable used in qmp_migrate() i.e.

'MigrationChannel *channel = NULL', is defined in qmp_migrate() and if the user opts for 'uri' then '@channels' coming from hmp_migrate() will be NULL, and then migrate_uri_parse() will populate memory into 'channel', and that is not getting freed after it's use is over. 

I think, that is where memory leak might be happening ?

Regards,

    Het Gala