[libvirt] [PATCH 6/6] qemu: enable monitor event filtering by name

Filtering monitor events by name requires tracking the name for the duration of the filtering. In order to free the name, I found it easiest to just piggyback on the user's freecb function, which gets called when the event is deregistered. For events without a name filter, we have the design of multiple client registrations sharing a common server registration, because the server side uses the same callback function and we reject duplicate use of the same function. But with events in the mix, we want to be able to allow the same function pointer to be used with more than one event name. The solution is to tweak the duplicate detection code to only act when there is no additional filtering; if name filtering is in use, there is exactly one client registration per server registration. Yes, this means that there is no longer a bound on the number of server registrations possible, so a malicious client could repeatedly register for the same name event to exhaust server memory. On the other hand, we already restricted monitor events to require write access (compared to normal events only needing read access), and separated it into the intentionally unsuported libvirt-qemu.so, with documentation that using this function is for debug purposes only; so it is not a security risk worth worrying about a client trying to abuse multiple registrations. * src/conf/domain_event.c (virDomainQemuMonitorEventData): New struct. (virDomainQemuMonitorEventFilter) (virDomainQemuMonitorEventCleanup): New functions. (virDomainQemuMonitorEventDispatchFunc) (virDomainQemuMonitorEventStateRegisterID): Use new struct. * src/conf/object_event.c (virObjectEventCallbackListCount) (virObjectEventCallbackListAddID) (virObjectEventCallbackListRemoveID) (virObjectEventCallbackListMarkDeleteID): Drop duplicate detection when filtering is in effect. Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- src/conf/domain_event.c | 71 +++++++++++++++++++++++++++++++++++++++++++------ src/conf/object_event.c | 40 ++++++++++++++++++---------- 2 files changed, 89 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c index 66fe769..1fb5243 100644 --- a/src/conf/domain_event.c +++ b/src/conf/domain_event.c @@ -1383,6 +1383,18 @@ error: } +/* In order to filter by event name, we need to store a copy of the + * name to filter on. By wrapping the caller's freecb, we can + * piggyback our cleanup to happen at the same time the caller + * deregisters. */ +struct virDomainQemuMonitorEventData { + char *event; + void *opaque; + virFreeCallback freecb; +}; +typedef struct virDomainQemuMonitorEventData virDomainQemuMonitorEventData; + + static void virDomainQemuMonitorEventDispatchFunc(virConnectPtr conn, virObjectEventPtr event, @@ -1391,6 +1403,7 @@ virDomainQemuMonitorEventDispatchFunc(virConnectPtr conn, { virDomainPtr dom = virGetDomain(conn, event->meta.name, event->meta.uuid); virDomainQemuMonitorEventPtr qemuMonitorEvent; + virDomainQemuMonitorEventData *data = cbopaque; if (!dom) return; @@ -1402,7 +1415,7 @@ virDomainQemuMonitorEventDispatchFunc(virConnectPtr conn, qemuMonitorEvent->seconds, qemuMonitorEvent->micros, qemuMonitorEvent->details, - cbopaque); + data->opaque); virDomainFree(dom); } @@ -1577,6 +1590,41 @@ virDomainEventStateDeregister(virConnectPtr conn, /** + * virDomainQemuMonitorEventFilter: + * @conn: the connection pointer + * @event: the event about to be dispatched + * @opaque: the opaque data registered with the filter + * + * Callback for filtering based on event names. Returns true if the + * event should be dispatched. + */ +static bool +virDomainQemuMonitorEventFilter(virConnectPtr conn ATTRIBUTE_UNUSED, + virObjectEventPtr event, + void *opaque) +{ + virDomainQemuMonitorEventData *data = opaque; + virDomainQemuMonitorEventPtr monitorEvent; + + monitorEvent = (virDomainQemuMonitorEventPtr) event; + + return STREQ(monitorEvent->event, data->event); +} + + +static void +virDomainQemuMonitorEventCleanup(void *opaque) +{ + virDomainQemuMonitorEventData *data = opaque; + + VIR_FREE(data->event); + if (data->freecb) + (data->freecb)(data->opaque); + VIR_FREE(data); +} + + +/** * virDomainQemuMonitorEventStateRegisterID: * @conn: connection to associate with callback * @state: object event state @@ -1602,21 +1650,28 @@ virDomainQemuMonitorEventStateRegisterID(virConnectPtr conn, virFreeCallback freecb, int *callbackID) { + virDomainQemuMonitorEventData *data = NULL; + virObjectEventCallbackFilter filter = NULL; + if (virDomainEventsInitialize() < 0) return -1; - /* FIXME support event filtering */ - if (event) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("event filtering on '%s' not implemented yet"), - event); + if (VIR_ALLOC(data) < 0) + return -1; + if (VIR_STRDUP(data->event, event) < 0) { + VIR_FREE(data); return -1; } + data->opaque = opaque; + data->freecb = freecb; + if (event) + filter = virDomainQemuMonitorEventFilter; + freecb = virDomainQemuMonitorEventCleanup; return virObjectEventStateRegisterID(conn, state, dom ? dom->uuid : NULL, - NULL, NULL, + filter, data, virDomainQemuMonitorEventClass, 0, VIR_OBJECT_EVENT_CALLBACK(cb), - opaque, freecb, + data, freecb, false, callbackID, false); } diff --git a/src/conf/object_event.c b/src/conf/object_event.c index de45257..56df806 100644 --- a/src/conf/object_event.c +++ b/src/conf/object_event.c @@ -181,6 +181,8 @@ virObjectEventCallbackListCount(virConnectPtr conn, for (i = 0; i < cbList->count; i++) { virObjectEventCallbackPtr cb = cbList->callbacks[i]; + if (cb->filter) + continue; if (cb->klass == klass && cb->eventID == eventID && cb->conn == conn && @@ -216,10 +218,11 @@ virObjectEventCallbackListRemoveID(virConnectPtr conn, if (cb->callbackID == callbackID && cb->conn == conn) { int ret; - ret = virObjectEventCallbackListCount(conn, cbList, cb->klass, - cb->eventID, - cb->uuid_filter ? cb->uuid : NULL, - cb->remoteID >= 0) - 1; + ret = cb->filter ? 0 : + (virObjectEventCallbackListCount(conn, cbList, cb->klass, + cb->eventID, + cb->uuid_filter ? cb->uuid : NULL, + cb->remoteID >= 0) - 1); if (cb->freecb) (*cb->freecb)(cb->opaque); @@ -249,10 +252,11 @@ virObjectEventCallbackListMarkDeleteID(virConnectPtr conn, if (cb->callbackID == callbackID && cb->conn == conn) { cb->deleted = true; - return virObjectEventCallbackListCount(conn, cbList, cb->klass, - cb->eventID, - cb->uuid_filter ? cb->uuid : NULL, - cb->remoteID >= 0); + return cb->filter ? 0 : + virObjectEventCallbackListCount(conn, cbList, cb->klass, + cb->eventID, + cb->uuid_filter ? cb->uuid : NULL, + cb->remoteID >= 0); } } @@ -396,8 +400,10 @@ virObjectEventCallbackListAddID(virConnectPtr conn, return -1; } - /* check if we already have this callback on our list */ - if (virObjectEventCallbackLookup(conn, cbList, uuid, + /* If there is no additional filtering, then check if we already + * have this callback on our list. */ + if (!filter && + virObjectEventCallbackLookup(conn, cbList, uuid, klass, eventID, callback, legacy, serverFilter ? &remoteID : NULL) != -1) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -430,10 +436,16 @@ virObjectEventCallbackListAddID(virConnectPtr conn, if (VIR_APPEND_ELEMENT(cbList->callbacks, cbList->count, event) < 0) goto cleanup; - ret = virObjectEventCallbackListCount(conn, cbList, klass, eventID, - uuid, serverFilter); - if (serverFilter && remoteID < 0) - ret++; + /* When additional filtering is being done, every client callback + * is matched to exactly one server callback. */ + if (filter) { + ret = 1; + } else { + ret = virObjectEventCallbackListCount(conn, cbList, klass, eventID, + uuid, serverFilter); + if (serverFilter && remoteID < 0) + ret++; + } cleanup: if (event) -- 1.8.5.3