* [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
* [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
* [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
* 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
* 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
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.