[libvirt] [PATCH] rpc: Segfaults and memory leak in virNetTLSContextNew function
Daniel P. Berrangé
berrange at redhat.com
Mon Apr 15 09:21:13 UTC 2019
On Fri, Apr 12, 2019 at 06:10:49PM +0200, Adrian Brzezinski wrote:
> Failed new gnutls context allocations in virNetTLSContextNew function
> results in double free and segfault. Occasional memory leaks may also
> occur. You can read detailed description at:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1699062
>
> Signed-off-by: Adrian Brzezinski <redhat at adrb.pl>
> ---
> docs/news.xml | 10 ++++++++++
> src/rpc/virnettlscontext.c | 6 ++++--
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/docs/news.xml b/docs/news.xml
> index 21807f2..f6157ec 100644
> --- a/docs/news.xml
> +++ b/docs/news.xml
> @@ -350,6 +350,16 @@
> <section title="Bug fixes">
> <change>
> <summary>
> + rpc: Segfaults and memory leak in virNetTLSContextNew function
> + </summary>
> + <description>
> + Failed new gnutls context allocations in virNetTLSContextNew function
> + results in double free and segfault. Occasional memory leaks may also
> + occur.
> + </description>
> + </change>
> + <change>
> + <summary>
> qemu: Use CAP_DAC_OVERRIDE during QEMU capabilities probing
> </summary>
> <description>
> diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c
> index 72e9ed9..8f6ec8f 100644
> --- a/src/rpc/virnettlscontext.c
> +++ b/src/rpc/virnettlscontext.c
> @@ -703,14 +703,14 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert,
> return NULL;
>
> if (VIR_STRDUP(ctxt->priority, priority) < 0)
> - goto error;
> + goto ctxt_init_error;
>
> err = gnutls_certificate_allocate_credentials(&ctxt->x509cred);
> if (err) {
> virReportError(VIR_ERR_SYSTEM_ERROR,
> _("Unable to allocate x509 credentials: %s"),
> gnutls_strerror(err));
> - goto error;
> + goto ctxt_init_error;
> }
>
> if (sanityCheckCert &&
> @@ -759,6 +759,8 @@ static virNetTLSContextPtr virNetTLSContextNew(const char *cacert,
> if (isServer)
> gnutls_dh_params_deinit(ctxt->dhParams);
> gnutls_certificate_free_credentials(ctxt->x509cred);
> + ctxt_init_error:
Having multiple label targets is not an attractive pattern. The core
problem here is that gnutls_certificate_free_credentials will
unconditionally dereference the credentials struct without checking
if it is NULL. We can avoid this by just adding a check
if (ctxt->x509cred)
gnutls_certificate_free_credentials(ctxt->x509cred);
> + if (ctxt->priority) VIR_FREE(ctxt->priority);
> VIR_FREE(ctxt);
> return NULL;
> }
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
More information about the libvir-list
mailing list