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@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 :|