External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2025-01-11 07:17:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

libxl: Fix races in libxl event code

Browse Source 
The libxl driver is racy in it's interactions with libxl and libvirt's
event loop.  The event loop can invoke callbacks after libxl has
deregistered the event, and possibly access freed data associated with
the event.

This patch fixes the race by converting libxlDomainObjPrivate to a
virObjectLockable, and locking it while executing libxl upcalls and
libvirt event loop callbacks.

Note that using the virDomainObj lock is not satisfactory since it may
be desirable to hold the virDomainObj lock even when libxl events such
as reading and writing to xenstore need processed.
This commit is contained in:
Jim Fehlig 2013-01-21 10:09:05 -07:00
parent 04172610c0
commit e0622ca281
2 changed files with 88 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/libxl/libxl_conf.h
View File

@ -1,5 +1,5 @@
/*---------------------------------------------------------------------------*/ /*---------------------------------------------------------------------------*/
/* Copyright (c) 2011 SUSE LINUX Products GmbH, Nuernberg, Germany. /* Copyright (C) 2011-2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
* Copyright (C) 2011 Univention GmbH. * Copyright (C) 2011 Univention GmbH.
* *
* This library is free software; you can redistribute it and/or * This library is free software; you can redistribute it and/or
@ -35,6 +35,7 @@
# include "capabilities.h" # include "capabilities.h"
# include "configmake.h" # include "configmake.h"
# include "virportallocator.h" # include "virportallocator.h"
# include "virobject.h"
# define LIBXL_VNC_PORT_MIN 5900 # define LIBXL_VNC_PORT_MIN 5900
@ -81,6 +82,8 @@ struct _libxlDriverPrivate {
typedef struct _libxlDomainObjPrivate libxlDomainObjPrivate; typedef struct _libxlDomainObjPrivate libxlDomainObjPrivate;
typedef libxlDomainObjPrivate *libxlDomainObjPrivatePtr; typedef libxlDomainObjPrivate *libxlDomainObjPrivatePtr;
struct _libxlDomainObjPrivate { struct _libxlDomainObjPrivate {
virObjectLockable parent;
/* per domain libxl ctx */ /* per domain libxl ctx */
libxl_ctx *ctx; libxl_ctx *ctx;
libxl_evgen_domain_death *deathW; libxl_evgen_domain_death *deathW;

140
src/libxl/libxl_driver.c
View File

@ -59,18 +59,16 @@
/* Number of Xen scheduler parameters */ /* Number of Xen scheduler parameters */
#define XEN_SCHED_CREDIT_NPARAM 2 #define XEN_SCHED_CREDIT_NPARAM 2
struct libxlOSEventHookFDInfo { /* Object used to store info related to libxl event registrations */
libxlDomainObjPrivatePtr priv; typedef struct _libxlEventHookInfo libxlEventHookInfo;
void *xl_priv; typedef libxlEventHookInfo *libxlEventHookInfoPtr;
int watch; struct _libxlEventHookInfo {
};
struct libxlOSEventHookTimerInfo {
libxlDomainObjPrivatePtr priv; libxlDomainObjPrivatePtr priv;
void *xl_priv; void *xl_priv;
int id; int id;
}; };
static virClassPtr libxlDomainObjPrivateClass;
static libxlDriverPrivatePtr libxl_driver = NULL; static libxlDriverPrivatePtr libxl_driver = NULL;
/* Function declarations */ /* Function declarations */
@ -84,6 +82,20 @@ libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
bool start_paused, int restore_fd); bool start_paused, int restore_fd);
/* Function definitions */ /* Function definitions */
static int
libxlDomainObjPrivateOnceInit(void)
{
if (!(libxlDomainObjPrivateClass = virClassNew(virClassForObjectLockable(),
"libxlDomainObjPrivate",
sizeof(libxlDomainObjPrivate),
NULL)))
return -1;
return 0;
}
VIR_ONCE_GLOBAL_INIT(libxlDomainObjPrivate)
static void static void
libxlDriverLock(libxlDriverPrivatePtr driver) libxlDriverLock(libxlDriverPrivatePtr driver)
{ {
@ -96,15 +108,22 @@ libxlDriverUnlock(libxlDriverPrivatePtr driver)
virMutexUnlock(&driver->lock); virMutexUnlock(&driver->lock);
} }
static void
libxlEventHookInfoFree(void *obj)
{
VIR_FREE(obj);
}
static void static void
libxlFDEventCallback(int watch ATTRIBUTE_UNUSED, libxlFDEventCallback(int watch ATTRIBUTE_UNUSED,
int fd, int fd,
int vir_events, int vir_events,
void *fd_info) void *fd_info)
{ {
struct libxlOSEventHookFDInfo *info = fd_info; libxlEventHookInfoPtr info = fd_info;
int events = 0; int events = 0;
virObjectLock(info->priv);
if (vir_events & VIR_EVENT_HANDLE_READABLE) if (vir_events & VIR_EVENT_HANDLE_READABLE)
events |= POLLIN; events |= POLLIN;
if (vir_events & VIR_EVENT_HANDLE_WRITABLE) if (vir_events & VIR_EVENT_HANDLE_WRITABLE)
@ -114,42 +133,37 @@ libxlFDEventCallback(int watch ATTRIBUTE_UNUSED,
if (vir_events & VIR_EVENT_HANDLE_HANGUP) if (vir_events & VIR_EVENT_HANDLE_HANGUP)
events |= POLLHUP; events |= POLLHUP;
virObjectUnlock(info->priv);
libxl_osevent_occurred_fd(info->priv->ctx, info->xl_priv, fd, 0, events); libxl_osevent_occurred_fd(info->priv->ctx, info->xl_priv, fd, 0, events);
} }
static void
libxlFreeFDInfo(void *obj)
{
VIR_FREE(obj);
}
static int static int
libxlFDRegisterEventHook(void *priv, int fd, void **hndp, libxlFDRegisterEventHook(void *priv, int fd, void **hndp,
short events, void *xl_priv) short events, void *xl_priv)
{ {
int vir_events = VIR_EVENT_HANDLE_ERROR; int vir_events = VIR_EVENT_HANDLE_ERROR;
struct libxlOSEventHookFDInfo *fdinfo; libxlEventHookInfoPtr info;
if (VIR_ALLOC(fdinfo) < 0) { if (VIR_ALLOC(info) < 0) {
virReportOOMError(); virReportOOMError();
return -1; return -1;
} }
fdinfo->priv = priv;
fdinfo->xl_priv = xl_priv;
*hndp = fdinfo;
if (events & POLLIN) if (events & POLLIN)
vir_events |= VIR_EVENT_HANDLE_READABLE; vir_events |= VIR_EVENT_HANDLE_READABLE;
if (events & POLLOUT) if (events & POLLOUT)
vir_events |= VIR_EVENT_HANDLE_WRITABLE; vir_events |= VIR_EVENT_HANDLE_WRITABLE;
fdinfo->watch = virEventAddHandle(fd, vir_events, libxlFDEventCallback, info->id = virEventAddHandle(fd, vir_events, libxlFDEventCallback,
fdinfo, libxlFreeFDInfo); info, libxlEventHookInfoFree);
if (fdinfo->watch < 0) { if (info->id < 0) {
VIR_FREE(fdinfo); VIR_FREE(info);
return fdinfo->watch; return -1;
} }
info->priv = priv;
info->xl_priv = xl_priv;
*hndp = info;
return 0; return 0;
} }
@ -159,15 +173,18 @@ libxlFDModifyEventHook(void *priv ATTRIBUTE_UNUSED,
void **hndp, void **hndp,
short events) short events)
{ {
struct libxlOSEventHookFDInfo *fdinfo = *hndp; libxlEventHookInfoPtr info = *hndp;
int vir_events = VIR_EVENT_HANDLE_ERROR; int vir_events = VIR_EVENT_HANDLE_ERROR;
virObjectLock(info->priv);
if (events & POLLIN) if (events & POLLIN)
vir_events |= VIR_EVENT_HANDLE_READABLE; vir_events |= VIR_EVENT_HANDLE_READABLE;
if (events & POLLOUT) if (events & POLLOUT)
vir_events |= VIR_EVENT_HANDLE_WRITABLE; vir_events |= VIR_EVENT_HANDLE_WRITABLE;
virEventUpdateHandle(fdinfo->watch, vir_events); virEventUpdateHandle(info->id, vir_events);
virObjectUnlock(info->priv);
return 0; return 0;
} }
@ -176,16 +193,21 @@ libxlFDDeregisterEventHook(void *priv ATTRIBUTE_UNUSED,
int fd ATTRIBUTE_UNUSED, int fd ATTRIBUTE_UNUSED,
void *hnd) void *hnd)
{ {
struct libxlOSEventHookFDInfo *fdinfo = hnd; libxlEventHookInfoPtr info = hnd;
libxlDomainObjPrivatePtr p = info->priv;
virEventRemoveHandle(fdinfo->watch); virObjectLock(p);
virEventRemoveHandle(info->id);
virObjectUnlock(p);
} }
static void static void
libxlTimerCallback(int timer ATTRIBUTE_UNUSED, void *timer_info) libxlTimerCallback(int timer ATTRIBUTE_UNUSED, void *timer_info)
{ {
struct libxlOSEventHookTimerInfo *info = timer_info; libxlEventHookInfoPtr info = timer_info;
libxlDomainObjPrivatePtr p = info->priv;
virObjectLock(p);
/* /*
* libxl expects the event to be deregistered when calling * libxl expects the event to be deregistered when calling
* libxl_osevent_occurred_timeout, but we dont want the event info * libxl_osevent_occurred_timeout, but we dont want the event info
@ -193,14 +215,11 @@ libxlTimerCallback(int timer ATTRIBUTE_UNUSED, void *timer_info)
* from libxl. * from libxl.
*/ */
virEventUpdateTimeout(info->id, -1); virEventUpdateTimeout(info->id, -1);
libxl_osevent_occurred_timeout(info->priv->ctx, info->xl_priv); virObjectUnlock(p);
libxl_osevent_occurred_timeout(p->ctx, info->xl_priv);
virObjectLock(p);
virEventRemoveTimeout(info->id); virEventRemoveTimeout(info->id);
} virObjectUnlock(p);
static void
libxlTimerInfoFree(void* obj)
{
VIR_FREE(obj);
} }
static int static int
@ -209,13 +228,13 @@ libxlTimeoutRegisterEventHook(void *priv,
struct timeval abs_t, struct timeval abs_t,
void *xl_priv) void *xl_priv)
{ {
libxlEventHookInfoPtr info;
struct timeval now; struct timeval now;
struct timeval res; struct timeval res;
static struct timeval zero; static struct timeval zero;
struct libxlOSEventHookTimerInfo *timer_info; int timeout;
int timeout, timer_id;
if (VIR_ALLOC(timer_info) < 0) { if (VIR_ALLOC(info) < 0) {
virReportOOMError(); virReportOOMError();
return -1; return -1;
} }
@ -230,17 +249,17 @@ libxlTimeoutRegisterEventHook(void *priv,
} else { } else {
timeout = res.tv_sec * 1000 + (res.tv_usec + 999) / 1000; timeout = res.tv_sec * 1000 + (res.tv_usec + 999) / 1000;
} }
timer_id = virEventAddTimeout(timeout, libxlTimerCallback, info->id = virEventAddTimeout(timeout, libxlTimerCallback,
timer_info, libxlTimerInfoFree); info, libxlEventHookInfoFree);
if (timer_id < 0) { if (info->id < 0) {
VIR_FREE(timer_info); VIR_FREE(info);
return timer_id; return -1;
} }
timer_info->priv = priv; info->priv = priv;
timer_info->xl_priv = xl_priv; info->xl_priv = xl_priv;
timer_info->id = timer_id; *hndp = info;
*hndp = timer_info;
return 0; return 0;
} }
@ -259,10 +278,13 @@ libxlTimeoutModifyEventHook(void *priv ATTRIBUTE_UNUSED,
void **hndp, void **hndp,
struct timeval abs_t ATTRIBUTE_UNUSED) struct timeval abs_t ATTRIBUTE_UNUSED)
{ {
struct libxlOSEventHookTimerInfo *timer_info = *hndp; libxlEventHookInfoPtr info = *hndp;
virObjectLock(info->priv);
/* Make the timeout fire */ /* Make the timeout fire */
virEventUpdateTimeout(timer_info->id, 0); virEventUpdateTimeout(info->id, 0);
virObjectUnlock(info->priv);
return 0; return 0;
} }
@ -270,9 +292,12 @@ static void
libxlTimeoutDeregisterEventHook(void *priv ATTRIBUTE_UNUSED, libxlTimeoutDeregisterEventHook(void *priv ATTRIBUTE_UNUSED,
void *hnd) void *hnd)
{ {
struct libxlOSEventHookTimerInfo *timer_info = hnd; libxlEventHookInfoPtr info = hnd;
libxlDomainObjPrivatePtr p = info->priv;
virEventRemoveTimeout(timer_info->id); virObjectLock(p);
virEventRemoveTimeout(info->id);
virObjectUnlock(p);
} }
static const libxl_osevent_hooks libxl_event_callbacks = { static const libxl_osevent_hooks libxl_event_callbacks = {
@ -289,12 +314,15 @@ libxlDomainObjPrivateAlloc(void)
{ {
libxlDomainObjPrivatePtr priv; libxlDomainObjPrivatePtr priv;
if (VIR_ALLOC(priv) < 0) if (libxlDomainObjPrivateInitialize() < 0)
return NULL;
if (!(priv = virObjectLockableNew(libxlDomainObjPrivateClass)))
return NULL; return NULL;
if (libxl_ctx_alloc(&priv->ctx, LIBXL_VERSION, 0, libxl_driver->logger)) { if (libxl_ctx_alloc(&priv->ctx, LIBXL_VERSION, 0, libxl_driver->logger)) {
VIR_ERROR(_("Failed libxl context initialization")); VIR_ERROR(_("Failed libxl context initialization"));
VIR_FREE(priv); virObjectUnref(priv);
return NULL; return NULL;
} }
@ -312,7 +340,7 @@ libxlDomainObjPrivateFree(void *data)
libxl_evdisable_domain_death(priv->ctx, priv->deathW); libxl_evdisable_domain_death(priv->ctx, priv->deathW);
libxl_ctx_free(priv->ctx); libxl_ctx_free(priv->ctx);
VIR_FREE(priv); virObjectUnref(priv);
} }
/* driver must be locked before calling */ /* driver must be locked before calling */