[libvirt] [PATCH] Fix event-test segfault for mixed remote/local driver usage
If examples/domain-events/events-c/event-test.c uses a local driver for the hyperviror and a remote driver for storage then the segfault occurs in remoteDomainEventDispatchFunc because conn->privateData is used. The problem is as follows. The doRemoteOpen function registers remoteDomainEventFired as event handle and remoteDomainEventQueueFlush as event timeout. The doRemoteOpen function is used in remoteOpen and remoteOpenSecondaryDriver. So both handlers are registered for all types of remote drivers, but remoteDomainEventQueueFlush always uses conn->privateData. That's wrong when remoteDomainEventQueueFlush is called for any other driver than the hypervisor driver, it should use conn->storagePrivateData for the storage driver for example. In the common case this doesn't result in a segfault because in the common case all drivers for a connection are either all remote or all local and conn->privateData and conn->storagePrivateData are the same. But in the local Xen case ('event-test xen:///' executed in Domain-0, with a libvirtd running in Domain-0) the hypervisor driver is local and the storage driver is remote. This results in a mismatch between conn->privateData and conn->storagePrivateData. Now the call to remoteDomainEventQueueFlush in the context of the storage driver results in a segfault, because it assumes that conn->privateData points to remote driver private data, but it points to Xen driver private data. To fix this, the pointer to the private driver data gets passed as opaque parameter instead of the connection pointer. Because some functions called by the event handler functions need the connection pointer for other purposes than error reporting the connection pointer is passed as member of the private remote driver data struct. Now remoteDomainEventFired and remoteDomainEventQueueFlush have the correct private data pointer for the respective calling context. --- src/remote/remote_driver.c | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index d6f5fce..582b85c 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -152,6 +152,7 @@ struct private_stream_data { struct private_data { virMutex lock; + virConnectPtr conn; int sock; /* Socket. */ int watch; /* File handle watch */ @@ -893,7 +894,7 @@ doRemoteOpen (virConnectPtr conn, if ((priv->watch = virEventAddHandle(priv->sock, VIR_EVENT_HANDLE_READABLE, remoteDomainEventFired, - conn, NULL)) < 0) { + priv, NULL)) < 0) { DEBUG0("virEventAddHandle failed: No addHandleImpl defined." " continuing without events."); } else { @@ -901,7 +902,7 @@ doRemoteOpen (virConnectPtr conn, DEBUG0("Adding Timeout for remote event queue flushing"); if ( (priv->eventFlushTimer = virEventAddTimeout(-1, remoteDomainEventQueueFlush, - conn, NULL)) < 0) { + priv, NULL)) < 0) { DEBUG0("virEventAddTimeout failed: No addTimeoutImpl defined. " "continuing without events."); virEventRemoveHandle(priv->watch); @@ -980,6 +981,7 @@ remoteAllocPrivateData(virConnectPtr conn) return NULL; } remoteDriverLock(priv); + priv->conn = conn; priv->localUses = 1; priv->watch = -1; priv->sock = -1; @@ -8659,8 +8661,7 @@ remoteDomainEventFired(int watch, int event, void *opaque) { - virConnectPtr conn = opaque; - struct private_data *priv = conn->privateData; + struct private_data *priv = opaque; remoteDriverLock(priv); @@ -8684,7 +8685,7 @@ remoteDomainEventFired(int watch, goto done; } - if (remoteIOHandleInput(conn, priv, 0) < 0) + if (remoteIOHandleInput(priv->conn, priv, 0) < 0) DEBUG0("Something went wrong during async message processing"); done: @@ -8708,8 +8709,7 @@ static void remoteDomainEventDispatchFunc(virConnectPtr conn, void remoteDomainEventQueueFlush(int timer ATTRIBUTE_UNUSED, void *opaque) { - virConnectPtr conn = opaque; - struct private_data *priv = conn->privateData; + struct private_data *priv = opaque; virDomainEventQueue tempQueue; remoteDriverLock(priv); @@ -8731,7 +8731,7 @@ remoteDomainEventQueueFlush(int timer ATTRIBUTE_UNUSED, void *opaque) if ( priv->callbackList->count == 0 ) { /* Tell the server when we are the last callback deregistering */ - if (call (conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER, + if (call (priv->conn, priv, 0, REMOTE_PROC_DOMAIN_EVENTS_DEREGISTER, (xdrproc_t) xdr_void, (char *) NULL, (xdrproc_t) xdr_void, (char *) NULL) == -1) VIR_WARN0("Failed to de-register events"); -- 1.6.3.3
On Thu, Jan 14, 2010 at 03:19:06AM +0100, Matthias Bolte wrote:
If examples/domain-events/events-c/event-test.c uses a local driver for the hyperviror and a remote driver for storage then the segfault occurs in remoteDomainEventDispatchFunc because conn->privateData is used.
The problem is as follows. The doRemoteOpen function registers remoteDomainEventFired as event handle and remoteDomainEventQueueFlush as event timeout. The doRemoteOpen function is used in remoteOpen and remoteOpenSecondaryDriver. So both handlers are registered for all types of remote drivers, but remoteDomainEventQueueFlush always uses conn->privateData. That's wrong when remoteDomainEventQueueFlush is called for any other driver than the hypervisor driver, it should use conn->storagePrivateData for the storage driver for example.
In the common case this doesn't result in a segfault because in the common case all drivers for a connection are either all remote or all local and conn->privateData and conn->storagePrivateData are the same. But in the local Xen case ('event-test xen:///' executed in Domain-0, with a libvirtd running in Domain-0) the hypervisor driver is local and the storage driver is remote. This results in a mismatch between conn->privateData and conn->storagePrivateData. Now the call to remoteDomainEventQueueFlush in the context of the storage driver results in a segfault, because it assumes that conn->privateData points to remote driver private data, but it points to Xen driver private data.
The big unanswered question here is why is the remote driver receiving any events at all in the first place ? Only the domain APIs emit events, and so those will all be coming out via the primary Xen driver. So even if the xen:/// URI driver uses the remote driver a secondary driver, it should never be receiving any events.
To fix this, the pointer to the private driver data gets passed as opaque parameter instead of the connection pointer. Because some functions called by the event handler functions need the connection pointer for other purposes than error reporting the connection pointer is passed as member of the private remote driver data struct.
Now remoteDomainEventFired and remoteDomainEventQueueFlush have the correct private data pointer for the respective calling context.
My inclination would be to skip registering the timer for the remoteDomainEventQueueFlush method. Then inthe code which recieves events, explicitly throw them away if eventFlushTimer == -1, and log a warning message about unexpected events. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (2)
-
Daniel P. Berrange -
Matthias Bolte