[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[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 redhat com>
---
 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]