* [PATCH v2 0/3] Fix memory leak of some device state in migration
@ 2020-12-28 9:00 g00517791
2020-12-28 9:00 ` [PATCH v2 1/3] spapr_pci: Fix memory leak of vmstate_spapr_pci g00517791
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: g00517791 @ 2020-12-28 9:00 UTC (permalink / raw)
To: qemu-ppc, qemu-devel
Cc: Stefan Berger, Jason Wang, Michael S . Tsirkin, Greg Kurz,
Dr . David Alan Gilbert, Juan Quintela, wanghaibin.wang,
Marc-André Lureau, zhukeqian1, David Gibson
From: Jinhao Gao <gaojinhao@huawei.com>
For some device state having some fields of VMS_ALLOC flag, they
don't free memory allocated for the fields in vmstate_save_state
and vmstate_load_state. We add funcs or sentences of free memory
before and after VM saves or loads device state to avoid memory leak.
v2
- Drop patch1-3,6-8 of v1
- Address Michael's comment (free memory before load vmsd centrally)
- Add David's Acked-by and Michael's Signed-off-by
Jinhao Gao (3):
spapr_pci: Fix memory leak of vmstate_spapr_pci
savevm: Fix memory leak of vmstate_configuration
vmstate: Fix memory leak in vmstate_handle_alloc()
hw/ppc/spapr_pci.c | 11 +++++++++++
migration/savevm.c | 31 +++++++++++++++++++++++++++----
migration/vmstate.c | 1 +
3 files changed, 39 insertions(+), 4 deletions(-)
--
2.23.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 1/3] spapr_pci: Fix memory leak of vmstate_spapr_pci 2020-12-28 9:00 [PATCH v2 0/3] Fix memory leak of some device state in migration g00517791 @ 2020-12-28 9:00 ` g00517791 2020-12-28 12:10 ` Michael S. Tsirkin 2020-12-28 9:00 ` [PATCH v2 2/3] savevm: Fix memory leak of vmstate_configuration g00517791 2020-12-28 9:00 ` [PATCH v2 3/3] vmstate: Fix memory leak in vmstate_handle_alloc() g00517791 2 siblings, 1 reply; 6+ messages in thread From: g00517791 @ 2020-12-28 9:00 UTC (permalink / raw) To: qemu-ppc, qemu-devel Cc: Stefan Berger, Jason Wang, Michael S . Tsirkin, Greg Kurz, Dr . David Alan Gilbert, Juan Quintela, wanghaibin.wang, Marc-André Lureau, zhukeqian1, David Gibson From: Jinhao Gao <gaojinhao@huawei.com> When VM migrate VMState of spapr_pci, the field(msi_devs) of spapr_pci having a flag of VMS_ALLOC need to allocate memory. If the src doesn't free memory of msi_devs in SaveStateEntry of spapr_pci after QEMUFile save VMState of spapr_pci, it may result in memory leak of msi_devs. We add the post_save 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_pci.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 76d7c91e9c..1b2b940606 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -2173,6 +2173,16 @@ static int spapr_pci_pre_save(void *opaque) return 0; } +static int spapr_pci_post_save(void *opaque) +{ + SpaprPhbState *sphb = opaque; + + g_free(sphb->msi_devs); + sphb->msi_devs = NULL; + sphb->msi_devs_num = 0; + return 0; +} + static int spapr_pci_post_load(void *opaque, int version_id) { SpaprPhbState *sphb = opaque; @@ -2205,6 +2215,7 @@ static const VMStateDescription vmstate_spapr_pci = { .version_id = 2, .minimum_version_id = 2, .pre_save = spapr_pci_pre_save, + .post_save = spapr_pci_post_save, .post_load = spapr_pci_post_load, .fields = (VMStateField[]) { VMSTATE_UINT64_EQUAL(buid, SpaprPhbState, NULL), -- 2.23.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/3] spapr_pci: Fix memory leak of vmstate_spapr_pci 2020-12-28 9:00 ` [PATCH v2 1/3] spapr_pci: Fix memory leak of vmstate_spapr_pci g00517791 @ 2020-12-28 12:10 ` Michael S. Tsirkin 0 siblings, 0 replies; 6+ messages in thread From: Michael S. Tsirkin @ 2020-12-28 12:10 UTC (permalink / raw) To: g00517791 Cc: Stefan Berger, Jason Wang, Juan Quintela, Greg Kurz, qemu-devel, qemu-ppc, wanghaibin.wang, Marc-André Lureau, zhukeqian1, Dr . David Alan Gilbert, David Gibson On Mon, Dec 28, 2020 at 05:00:51PM +0800, g00517791 wrote: > From: Jinhao Gao <gaojinhao@huawei.com> > > When VM migrate VMState of spapr_pci, the field(msi_devs) of spapr_pci > having a flag of VMS_ALLOC need to allocate memory. If the src doesn't free > memory of msi_devs in SaveStateEntry of spapr_pci after QEMUFile save > VMState of spapr_pci, it may result in memory leak of msi_devs. We add the > post_save 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> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > --- > hw/ppc/spapr_pci.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 76d7c91e9c..1b2b940606 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -2173,6 +2173,16 @@ static int spapr_pci_pre_save(void *opaque) > return 0; > } > > +static int spapr_pci_post_save(void *opaque) > +{ > + SpaprPhbState *sphb = opaque; > + > + g_free(sphb->msi_devs); > + sphb->msi_devs = NULL; > + sphb->msi_devs_num = 0; > + return 0; > +} > + > static int spapr_pci_post_load(void *opaque, int version_id) > { > SpaprPhbState *sphb = opaque; > @@ -2205,6 +2215,7 @@ static const VMStateDescription vmstate_spapr_pci = { > .version_id = 2, > .minimum_version_id = 2, > .pre_save = spapr_pci_pre_save, > + .post_save = spapr_pci_post_save, > .post_load = spapr_pci_post_load, > .fields = (VMStateField[]) { > VMSTATE_UINT64_EQUAL(buid, SpaprPhbState, NULL), > -- > 2.23.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] savevm: Fix memory leak of vmstate_configuration 2020-12-28 9:00 [PATCH v2 0/3] Fix memory leak of some device state in migration g00517791 2020-12-28 9:00 ` [PATCH v2 1/3] spapr_pci: Fix memory leak of vmstate_spapr_pci g00517791 @ 2020-12-28 9:00 ` g00517791 2020-12-28 12:10 ` Michael S. Tsirkin 2020-12-28 9:00 ` [PATCH v2 3/3] vmstate: Fix memory leak in vmstate_handle_alloc() g00517791 2 siblings, 1 reply; 6+ messages in thread From: g00517791 @ 2020-12-28 9:00 UTC (permalink / raw) To: qemu-ppc, qemu-devel Cc: Stefan Berger, Jason Wang, Michael S . Tsirkin, Greg Kurz, Dr . David Alan Gilbert, Juan Quintela, wanghaibin.wang, Marc-André Lureau, zhukeqian1, David Gibson From: Jinhao Gao <gaojinhao@huawei.com> When VM migrate VMState of configuration, the fields(name and capabilities) of configuration having a flag of VMS_ALLOC need to allocate memory. If the src doesn't free memory of capabilities in SaveState after save VMState of configuration, or the dst doesn't free memory of name and capabilities in post load of configuration, it may result in memory leak of name and capabilities. We free memory in configuration_post_save and configuration_post_load func, which prevents memory leak. Signed-off-by: Jinhao Gao <gaojinhao@huawei.com> --- migration/savevm.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) diff --git a/migration/savevm.c b/migration/savevm.c index 5f937a2762..13f1a5dab7 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -314,6 +314,16 @@ static int configuration_pre_save(void *opaque) return 0; } +static int configuration_post_save(void *opaque) +{ + SaveState *state = opaque; + + g_free(state->capabilities); + state->capabilities = NULL; + state->caps_count = 0; + return 0; +} + static int configuration_pre_load(void *opaque) { SaveState *state = opaque; @@ -364,24 +374,36 @@ static int configuration_post_load(void *opaque, int version_id) { SaveState *state = opaque; const char *current_name = MACHINE_GET_CLASS(current_machine)->name; + int ret = 0; if (strncmp(state->name, current_name, state->len) != 0) { error_report("Machine type received is '%.*s' and local is '%s'", (int) state->len, state->name, current_name); - return -EINVAL; + ret = -EINVAL; + goto out; } if (state->target_page_bits != qemu_target_page_bits()) { error_report("Received TARGET_PAGE_BITS is %d but local is %d", state->target_page_bits, qemu_target_page_bits()); - return -EINVAL; + ret = -EINVAL; + goto out; } if (!configuration_validate_capabilities(state)) { - return -EINVAL; + ret = -EINVAL; + goto out; } - return 0; +out: + g_free((void *)state->name); + state->name = NULL; + state->len = 0; + g_free(state->capabilities); + state->capabilities = NULL; + state->caps_count = 0; + + return ret; } static int get_capability(QEMUFile *f, void *pv, size_t size, @@ -515,6 +537,7 @@ static const VMStateDescription vmstate_configuration = { .pre_load = configuration_pre_load, .post_load = configuration_post_load, .pre_save = configuration_pre_save, + .post_save = configuration_post_save, .fields = (VMStateField[]) { VMSTATE_UINT32(len, SaveState), VMSTATE_VBUFFER_ALLOC_UINT32(name, SaveState, 0, NULL, len), -- 2.23.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/3] savevm: Fix memory leak of vmstate_configuration 2020-12-28 9:00 ` [PATCH v2 2/3] savevm: Fix memory leak of vmstate_configuration g00517791 @ 2020-12-28 12:10 ` Michael S. Tsirkin 0 siblings, 0 replies; 6+ messages in thread From: Michael S. Tsirkin @ 2020-12-28 12:10 UTC (permalink / raw) To: g00517791 Cc: Stefan Berger, Jason Wang, Juan Quintela, Greg Kurz, qemu-devel, qemu-ppc, wanghaibin.wang, Marc-André Lureau, zhukeqian1, Dr . David Alan Gilbert, David Gibson On Mon, Dec 28, 2020 at 05:00:52PM +0800, g00517791 wrote: > From: Jinhao Gao <gaojinhao@huawei.com> > > When VM migrate VMState of configuration, the fields(name and capabilities) > of configuration having a flag of VMS_ALLOC need to allocate memory. If the > src doesn't free memory of capabilities in SaveState after save VMState of > configuration, or the dst doesn't free memory of name and capabilities in post > load of configuration, it may result in memory leak of name and capabilities. > We free memory in configuration_post_save and configuration_post_load func, > which prevents memory leak. > > Signed-off-by: Jinhao Gao <gaojinhao@huawei.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > --- > migration/savevm.c | 31 +++++++++++++++++++++++++++---- > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/migration/savevm.c b/migration/savevm.c > index 5f937a2762..13f1a5dab7 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -314,6 +314,16 @@ static int configuration_pre_save(void *opaque) > return 0; > } > > +static int configuration_post_save(void *opaque) > +{ > + SaveState *state = opaque; > + > + g_free(state->capabilities); > + state->capabilities = NULL; > + state->caps_count = 0; > + return 0; > +} > + > static int configuration_pre_load(void *opaque) > { > SaveState *state = opaque; > @@ -364,24 +374,36 @@ static int configuration_post_load(void *opaque, int version_id) > { > SaveState *state = opaque; > const char *current_name = MACHINE_GET_CLASS(current_machine)->name; > + int ret = 0; > > if (strncmp(state->name, current_name, state->len) != 0) { > error_report("Machine type received is '%.*s' and local is '%s'", > (int) state->len, state->name, current_name); > - return -EINVAL; > + ret = -EINVAL; > + goto out; > } > > if (state->target_page_bits != qemu_target_page_bits()) { > error_report("Received TARGET_PAGE_BITS is %d but local is %d", > state->target_page_bits, qemu_target_page_bits()); > - return -EINVAL; > + ret = -EINVAL; > + goto out; > } > > if (!configuration_validate_capabilities(state)) { > - return -EINVAL; > + ret = -EINVAL; > + goto out; > } > > - return 0; > +out: > + g_free((void *)state->name); > + state->name = NULL; > + state->len = 0; > + g_free(state->capabilities); > + state->capabilities = NULL; > + state->caps_count = 0; > + > + return ret; > } > > static int get_capability(QEMUFile *f, void *pv, size_t size, > @@ -515,6 +537,7 @@ static const VMStateDescription vmstate_configuration = { > .pre_load = configuration_pre_load, > .post_load = configuration_post_load, > .pre_save = configuration_pre_save, > + .post_save = configuration_post_save, > .fields = (VMStateField[]) { > VMSTATE_UINT32(len, SaveState), > VMSTATE_VBUFFER_ALLOC_UINT32(name, SaveState, 0, NULL, len), > -- > 2.23.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] vmstate: Fix memory leak in vmstate_handle_alloc() 2020-12-28 9:00 [PATCH v2 0/3] Fix memory leak of some device state in migration g00517791 2020-12-28 9:00 ` [PATCH v2 1/3] spapr_pci: Fix memory leak of vmstate_spapr_pci g00517791 2020-12-28 9:00 ` [PATCH v2 2/3] savevm: Fix memory leak of vmstate_configuration g00517791 @ 2020-12-28 9:00 ` g00517791 2 siblings, 0 replies; 6+ messages in thread From: g00517791 @ 2020-12-28 9:00 UTC (permalink / raw) To: qemu-ppc, qemu-devel Cc: Stefan Berger, Jason Wang, Michael S . Tsirkin, Greg Kurz, Dr . David Alan Gilbert, Juan Quintela, wanghaibin.wang, Marc-André Lureau, zhukeqian1, David Gibson From: Jinhao Gao <gaojinhao@huawei.com> Some memory allocated for fields having a flag of VMS_ALLOC in SaveState may not free before VM load vmsd in migration. So we pre-free memory before allocation in vmstate_handle_alloc() to avoid memleaks. Signed-off-by: Jinhao Gao <gaojinhao@huawei.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- migration/vmstate.c | 1 + 1 file changed, 1 insertion(+) diff --git a/migration/vmstate.c b/migration/vmstate.c index e9d2aef66b..873f76739f 100644 --- a/migration/vmstate.c +++ b/migration/vmstate.c @@ -70,6 +70,7 @@ static void vmstate_handle_alloc(void *ptr, const VMStateField *field, gsize size = vmstate_size(opaque, field); size *= vmstate_n_elems(opaque, field); if (size) { + g_free(*(void **)ptr); *(void **)ptr = g_malloc(size); } } -- 2.23.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-28 12:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-12-28 9:00 [PATCH v2 0/3] Fix memory leak of some device state in migration g00517791 2020-12-28 9:00 ` [PATCH v2 1/3] spapr_pci: Fix memory leak of vmstate_spapr_pci g00517791 2020-12-28 12:10 ` Michael S. Tsirkin 2020-12-28 9:00 ` [PATCH v2 2/3] savevm: Fix memory leak of vmstate_configuration g00517791 2020-12-28 12:10 ` Michael S. Tsirkin 2020-12-28 9:00 ` [PATCH v2 3/3] vmstate: Fix memory leak in vmstate_handle_alloc() g00517791
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).