[PATCH v2 7/8] util: Add magic number check for object validity

[John Ferlan <jferlan@xxxxxxxxxx>]

The virObjectIsClass API has only ever checked object validity
based on if the @obj is not NULL and it was derived from some class.
While this has worked well in general, there is one additional
check that could be made prior to calling virClassIsDerivedFrom
which loops through the classes checking the magic number against
the klass expected magic number.

If by chance a non virObject is passed, rather than assuming the
void * @obj is a _virObject and thus offsetting to obj->klass,
obj->magic, and obj->parent, let's check that the void * @obj
has at least the "base part" of the magic number in the right
place and generate a more specific VIR_WARN message if not.

There are many consumers to virObjectIsClass, include the locking
primitives virObject{Lock|Unlock}, virObjectRWLock{Read|Write},
and virObjectRWUnlock. For those callers, the locking call will
not fail, but it also will not attempt a virMutex* call which
will "most likely" fail since the &obj->lock is used.

In order to avoid some possible future wrap on the 0xCAFExxxx
value, add a check during initialization that some new class
won't cause the wrap. Should be good for a few years at least!

It is still left up to the caller to handle the failed API calls
just as it would be if it passed a NULL opaque pointer anyobj.

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---
 src/util/virobject.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/src/util/virobject.c b/src/util/virobject.c
index af3f252..54d78b0 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -47,14 +47,21 @@ struct _virClass {
     virObjectDisposeCallback dispose;
 };
 
+#define VIR_OBJECT_NOTVALID(obj) (!obj || ((obj->u.s.magic & 0xFFFF0000) != 0xCAFE0000))
+
 #define VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, objclass)                    \
     do {                                                                    \
         virObjectPtr obj = anyobj;                                          \
-        if (!obj)                                                           \
-            VIR_WARN("Object cannot be NULL");                              \
-        else                                                                \
+        if (VIR_OBJECT_NOTVALID(obj)) {                                     \
+            if (!obj)                                                       \
+                VIR_WARN("Object cannot be NULL");                          \
+            else                                                            \
+                VIR_WARN("Object %p has a bad magic number %X",             \
+                         obj, obj->u.s.magic);                              \
+        } else {                                                            \
             VIR_WARN("Object %p (%s) is not a %s instance",                 \
                       anyobj, obj->klass->name, #objclass);                 \
+        }                                                                   \
     } while (0)
 
 
@@ -177,9 +184,14 @@ virClassNew(virClassPtr parent,
         goto error;
 
     klass->parent = parent;
+    klass->magic = virAtomicIntInc(&magicCounter);
+    if (klass->magic > 0xCAFEFFFF) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("too many object classes defined"));
+        goto error;
+    }
     if (VIR_STRDUP(klass->name, name) < 0)
         goto error;
-    klass->magic = virAtomicIntInc(&magicCounter);
     klass->objectSize = objectSize;
     klass->dispose = dispose;
 
@@ -535,7 +547,7 @@ virObjectIsClass(void *anyobj,
                  virClassPtr klass)
 {
     virObjectPtr obj = anyobj;
-    if (!obj)
+    if (VIR_OBJECT_NOTVALID(obj))
         return false;
 
     return virClassIsDerivedFrom(obj->klass, klass);
-- 
2.9.4

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list