[libvirt] [PATCH] libxl: fix libvirtd segfault

[Jim Fehlig]

Dave Allan wrote:
> Functional ack; this patch fixes the crash on my system.
>   

I've pushed this patch since it fixes a crash affecting everyone that
has built the libxl driver but not actually running Xen.  Any additional
review comments can be addressed with a follow-up patch.

Regards,
Jim

> On Fri, Aug 09, 2013 at 05:58:45PM -0600, Jim Fehlig wrote:
>   
>> Commit d72ef888 introduced a bug in the libxl driver that will
>> segfault libvirtd if libxl reports an error message, e.g. when
>> attempting to initialize the driver on a non-Xen system.  I
>> assumed it was valid to pass a NULL logger to libxl_ctx_alloc(),
>> but that is not the case since any errors associated with the ctx
>> that are emitted by libxl will dereference the logger and crash
>> libvirtd.
>>
>> Errors associated with the libxl driver-wide ctx could be useful
>> for debugging anyway, so create a 'libxl-driver.log' to capture
>> these errors.
>> ---
>>  src/libxl/libxl_conf.h   |  3 +++
>>  src/libxl/libxl_driver.c | 30 ++++++++++++++++++++++++++++--
>>  2 files changed, 31 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
>> index 78133b9..9d72b72 100644
>> --- a/src/libxl/libxl_conf.h
>> +++ b/src/libxl/libxl_conf.h
>> @@ -57,6 +57,9 @@ struct _libxlDriverPrivate {
>>      virDomainXMLOptionPtr xmlopt;
>>      unsigned int version;
>>  
>> +    /* log stream for driver-wide libxl ctx */
>> +    FILE *logger_file;
>> +    xentoollog_logger *logger;
>>      /* libxl ctx for driver wide ops; getVersion, getNodeInfo, ... */
>>      libxl_ctx *ctx;
>>  
>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
>> index cc9e772..9dc7261 100644
>> --- a/src/libxl/libxl_driver.c
>> +++ b/src/libxl/libxl_driver.c
>> @@ -1180,6 +1180,9 @@ libxlStateCleanup(void)
>>      virObjectUnref(libxl_driver->xmlopt);
>>      virObjectUnref(libxl_driver->domains);
>>      libxl_ctx_free(libxl_driver->ctx);
>> +    xtl_logger_destroy(libxl_driver->logger);
>> +    if (libxl_driver->logger_file)
>> +        VIR_FORCE_FCLOSE(libxl_driver->logger_file);
>>  
>>      virObjectUnref(libxl_driver->reservedVNCPorts);
>>  
>> @@ -1229,6 +1232,7 @@ libxlStateInitialize(bool privileged,
>>                       void *opaque ATTRIBUTE_UNUSED)
>>  {
>>      const libxl_version_info *ver_info;
>> +    char *log_file = NULL;
>>      virCommandPtr cmd;
>>      int status, ret = 0;
>>      unsigned int free_mem;
>> @@ -1308,6 +1312,17 @@ libxlStateInitialize(bool privileged,
>>          goto error;
>>      }
>>  
>> +    if (virAsprintf(&log_file, "%s/libxl-driver.log", libxl_driver->logDir) < 0)
>> +        goto error;
>> +
>> +    if ((libxl_driver->logger_file = fopen(log_file, "a")) == NULL)  {
>> +        virReportSystemError(errno,
>> +                             _("failed to create logfile %s"),
>> +                             log_file);
>> +        goto error;
>> +    }
>> +    VIR_FREE(log_file);
>> +
>>      /* read the host sysinfo */
>>      if (privileged)
>>          libxl_driver->hostsysinfo = virSysinfoRead();
>> @@ -1316,8 +1331,18 @@ libxlStateInitialize(bool privileged,
>>      if (!libxl_driver->domainEventState)
>>          goto error;
>>  
>> -    if (libxl_ctx_alloc(&libxl_driver->ctx, LIBXL_VERSION, 0, NULL)) {
>> -        VIR_INFO("cannot initialize libxenlight context, probably not running in a Xen Dom0, disabling driver");
>> +    libxl_driver->logger =
>> +            (xentoollog_logger *)xtl_createlogger_stdiostream(libxl_driver->logger_file, XTL_DEBUG, 0);
>> +    if (!libxl_driver->logger) {
>> +        VIR_INFO("cannot create logger for libxenlight, disabling driver");
>> +        goto fail;
>> +    }
>> +
>> +    if (libxl_ctx_alloc(&libxl_driver->ctx,
>> +                       LIBXL_VERSION, 0,
>> +                       libxl_driver->logger)) {
>> +        VIR_INFO("cannot initialize libxenlight context, probably not running "
>> +                 "in a Xen Dom0, disabling driver");
>>          goto fail;
>>      }
>>  
>> @@ -1383,6 +1408,7 @@ libxlStateInitialize(bool privileged,
>>  error:
>>      ret = -1;
>>  fail:
>> +    VIR_FREE(log_file);
>>      if (libxl_driver)
>>          libxlDriverUnlock(libxl_driver);
>>      libxlStateCleanup();
>>