Re: [Qemu-devel] [PULL 22/22] sev/i386: add sev_get_capabilities()

[Peter Maydell]

On 13 March 2018 at 12:56, Paolo Bonzini <address@hidden> wrote:
> From: Brijesh Singh <address@hidden>
>
> The function can be used to get the current SEV capabilities.
> The capabilities include platform diffie-hellman key (pdh) and certificate
> chain. The key can be provided to the external entities which wants to
> establish a trusted channel between SEV firmware and guest owner.

Hi; Coverity points out a resource leak in this function.

> +SevCapability *
> +sev_get_capabilities(void)
> +{
> +    SevCapability *cap;
> +    guchar *pdh_data, *cert_chain_data;
> +    size_t pdh_len = 0, cert_chain_len = 0;
> +    uint32_t ebx;
> +    int fd;
> +
> +    fd = open(DEFAULT_SEV_DEVICE, O_RDWR);
> +    if (fd < 0) {
> +        error_report("%s: Failed to open %s '%s'", __func__,
> +                     DEFAULT_SEV_DEVICE, strerror(errno));
> +        return NULL;
> +    }
> +
> +    if (sev_get_pdh_info(fd, &pdh_data, &pdh_len,
> +                         &cert_chain_data, &cert_chain_len)) {
> +        return NULL;

CID 1390570 says that in this error-return path we leak
fd(), because we never close it.

> +    }
> +
> +    cap = g_new0(SevCapability, 1);
> +    cap->pdh = g_base64_encode(pdh_data, pdh_len);
> +    cap->cert_chain = g_base64_encode(cert_chain_data, cert_chain_len);
> +
> +    host_cpuid(0x8000001F, 0, NULL, &ebx, NULL, NULL);
> +    cap->cbitpos = ebx & 0x3f;
> +
> +    /*
> +     * When SEV feature is enabled, we loose one bit in guest physical
> +     * addressing.
> +     */
> +    cap->reduced_phys_bits = 1;
> +
> +    g_free(pdh_data);
> +    g_free(cert_chain_data);
> +
> +    close(fd);
> +    return cap;
> +}

thanks
-- PMM