Re: [PATCH 3/8] spapr: Fix memory leak of vmstate_spapr_event_entry

From: David Gibson <david@gibson.dropbear.id.au>
To: g00517791 <gaojinhao@huawei.com>
Cc: "Michael S . Tsirkin" <mst@redhat.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Stefan Berger" <stefanb@linux.vnet.ibm.com>,
	qemu-devel@nongnu.org, "Greg Kurz" <groug@kaod.org>,
	"Juan Quintela" <quintela@redhat.com>,
	qemu-ppc@nongnu.org, wanghaibin.wang@huawei.com,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>,
	zhukeqian1@huawei.com,
	"Dr . David Alan Gilbert" <dgilbert@redhat.com>
Subject: Re: [PATCH 3/8] spapr: Fix memory leak of vmstate_spapr_event_entry
Date: Mon, 28 Dec 2020 17:56:35 +1100	[thread overview]
Message-ID: <20201228065635.GA6952@yekko.fritz.box> (raw)
In-Reply-To: <20201226103347.868-4-gaojinhao@huawei.com>

[-- Attachment #1: Type: text/plain, Size: 1861 bytes --]

On Sat, Dec 26, 2020 at 06:33:42PM +0800, g00517791 wrote:
> From: Jinhao Gao <gaojinhao@huawei.com>
> 
> When VM migrate VMState of spapr_event_log_entry, the field(extended_log)
> of spapr_event_log_entry having a flag of VMS_ALLOC needs to allocate
> memory. If the dst doesn't free memory which has been allocated for
> SaveStateEntry of spapr_event_log_entry before dst loads device state,
> it may result that the pointer of extended_log is overlaid when vm loads.
> We add the pre_load func to free memory, which prevents memory leak.
> 
> Signed-off-by: Jinhao Gao <gaojinhao@huawei.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/ppc/spapr.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 489cefcb81..ddfed1e7ca 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1799,10 +1799,22 @@ static bool spapr_pending_events_needed(void *opaque)
>      return !QTAILQ_EMPTY(&spapr->pending_events);
>  }
>  
> +static int spapr_event_log_entry_pre_load(void *opaque)
> +{
> +    SpaprEventLogEntry *entry = opaque;
> +
> +    g_free(entry->extended_log);
> +    entry->extended_log = NULL;
> +    entry->extended_length = 0;
> +
> +    return 0;
> +}
> +
>  static const VMStateDescription vmstate_spapr_event_entry = {
>      .name = "spapr_event_log_entry",
>      .version_id = 1,
>      .minimum_version_id = 1,
> +    .pre_load = spapr_event_log_entry_pre_load,
>      .fields = (VMStateField[]) {
>          VMSTATE_UINT32(summary, SpaprEventLogEntry),
>          VMSTATE_UINT32(extended_length, SpaprEventLogEntry),

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]