[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/3] savevm: Fix memory leak of vmstate_configuration
From: |
Michael S. Tsirkin |
Subject: |
Re: [PATCH v2 2/3] savevm: Fix memory leak of vmstate_configuration |
Date: |
Mon, 28 Dec 2020 07:10:45 -0500 |
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