[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 10/22] sev/i386: add command to initialize the me
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PULL 10/22] sev/i386: add command to initialize the memory encryption context |
Date: |
Fri, 27 Apr 2018 14:01:24 +0100 |
On 13 March 2018 at 12:56, Paolo Bonzini <address@hidden> wrote:
> From: Brijesh Singh <address@hidden>
>
> When memory encryption is enabled, KVM_SEV_INIT command is used to
> initialize the platform. The command loads the SEV related persistent
> data from non-volatile storage and initializes the platform context.
> This command should be first issued before invoking any other guest
> commands provided by the SEV firmware.
Hi; Coverity points out a memory leak in this code
(CID 1390613):
> +void *
> +sev_guest_init(const char *id)
> +{
> + SEVState *s;
> + char *devname;
> + int ret, fw_error;
> + uint32_t ebx;
> + uint32_t host_cbitpos;
> + struct sev_user_data_status status = {};
> +
> + s = g_new0(SEVState, 1);
Here we allocate memory into 's'...
> + s->sev_info = lookup_sev_guest_info(id);
> + if (!s->sev_info) {
> + error_report("%s: '%s' is not a valid '%s' object",
> + __func__, id, TYPE_QSEV_GUEST_INFO);
> + goto err;
...and this error-exit path will not free it because
it does g_free(sev_state), not g_free(s).
> + }
> +
> + sev_state = s;
> + s->state = SEV_STATE_UNINIT;
[...]
> +err:
> + g_free(sev_state);
> + sev_state = NULL;
> + return NULL;
> +}
The simplest fix is probably to move the 'sev_state = s;
s->state = SEV_STATE_UNINIT;' assignments to earlier in the
function.
Cleaner might be to ensure that nothing in the rest of
the function depends on sev_state being initialized,
and then to work purely with 's' and only set sev_state
to s just before returning success.
thanks
-- PMM
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PULL 10/22] sev/i386: add command to initialize the memory encryption context,
Peter Maydell <=