The virObject logic "assumes" that whatever is passed to its API's
would be some sort of virObjectPtr; however, if it is not then some
really bad things can happen.
So far there's been only virObject{Ref|Unref}, virObject{Lock|Unlock},
and virObjectIsClass and the virObject and virObjectLockable class
consumers have been well behaved and code well tested. Soon there will
be more consumers and one such consumer tripped over this during testing
by passing a virHashTablePtr to virObjectIsClass which ends up calling
virClassIsDerivedFrom using "obj->klass", which wasn't really a klass
object causing one of those bad things to happen.
To avoid the future possibility that a non virObject class memory was
passed to some virObject* API, let's add a "magic_marker" to the base
virObject class that contains a value "0xFEEDBEEF", then any place in
the code which would accept or process the base opaque virObjectPtr,
compare the magic_marker against 0xFEEDBEEF to make sure this is an
object allocated by this code.
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(a)redhat.com>
---
src/util/virobject.c | 12 ++++++++----
src/util/virobject.h | 1 +
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 9fe4e71..aea8a6d 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -47,10 +47,12 @@ struct _virClass {
virObjectDisposeCallback dispose;
};
+#define VIR_OBJECT_NOTVALID(obj) (!obj || obj->magic_marker != 0xFEEDBEEF)
+
#define VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, objclass) \
do { \
virObjectPtr obj = anyobj; \
- if (!obj) \
+ if (VIR_OBJECT_NOTVALID(obj)) \
VIR_WARN("Object %p is not a virObject class instance", anyobj);\
else \
VIR_WARN("Object %p (%s) is not a %s instance", \
@@ -212,6 +214,7 @@ virObjectNew(virClassPtr klass)
return NULL;
obj->u.s.magic = klass->magic;
+ obj->magic_marker = 0xFEEDBEEF;
obj->klass = klass;
virAtomicIntSet(&obj->u.s.refs, 1);
@@ -272,7 +275,7 @@ virObjectUnref(void *anyobj)
{
virObjectPtr obj = anyobj;
- if (!obj)
+ if (VIR_OBJECT_NOTVALID(obj))
return false;
bool lastRef = virAtomicIntDecAndTest(&obj->u.s.refs);
@@ -289,6 +292,7 @@ virObjectUnref(void *anyobj)
/* Clear & poison object */
memset(obj, 0, obj->klass->objectSize);
obj->u.s.magic = 0xDEADBEEF;
+ obj->magic_marker = 0xDEADBEEF;
obj->klass = (void*)0xDEADBEEF;
VIR_FREE(obj);
}
@@ -311,7 +315,7 @@ virObjectRef(void *anyobj)
{
virObjectPtr obj = anyobj;
- if (!obj)
+ if (VIR_OBJECT_NOTVALID(obj))
return NULL;
virAtomicIntInc(&obj->u.s.refs);
PROBE(OBJECT_REF, "obj=%p", obj);
@@ -390,7 +394,7 @@ virObjectIsClass(void *anyobj,
virClassPtr klass)
{
virObjectPtr obj = anyobj;
- if (!obj)
+ if (VIR_OBJECT_NOTVALID(obj))
return false;
return virClassIsDerivedFrom(obj->klass, klass);
diff --git a/src/util/virobject.h b/src/util/virobject.h
index f4c292b..89f8050 100644
--- a/src/util/virobject.h
+++ b/src/util/virobject.h
@@ -51,6 +51,7 @@ struct _virObject {
int refs;
} s;
} u;
+ unsigned int magic_marker;
virClassPtr klass;
};
--
2.9.3