aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMichael S. Tsirkin <mst@redhat.com>2022-02-25 08:40:27 -0500
committerMichael S. Tsirkin <mst@redhat.com>2022-03-04 05:40:58 -0500
commite36828787f2c35df64448f4142169ef5e193f5b3 (patch)
tree65f5e383a5b5e9129abf075bd6f68752bfea03dc
parent6629bf78aac7e53f83fd0bcbdbe322e2302dfd1f (diff)
qom: assert integer does not overflow
QOM reference counting is not designed with an infinite amount of references in mind, trying to take a reference in a loop without dropping a reference will overflow the integer. It is generally a symptom of a reference leak (a missing deref, commonly as part of error handling - such as one fixed here: https://lore.kernel.org/r/20220228095058.27899-1-sgarzare%40redhat.com ). All this can lead to either freeing the object too early (memory corruption) or never freeing it (memory leak). If we happen to dereference at just the right time (when it's wrapping around to 0), we might eventually assert when dereferencing, but the real problem is an extra object_ref so let's assert there to make such issues cleaner and easier to debug. Some micro-benchmarking shows using fetch and add this is essentially free on x86. Since multiple threads could be incrementing in parallel, we assert around INT_MAX to make sure none of these approach the wrap around point: this way we get a memory leak and not a memory corruption, the former is generally easier to debug. Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
-rw-r--r--qom/object.c6
1 files changed, 5 insertions, 1 deletions
diff --git a/qom/object.c b/qom/object.c
index 9f7a33139d..a27532a6ba 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1167,10 +1167,14 @@ GSList *object_class_get_list_sorted(const char *implements_type,
Object *object_ref(void *objptr)
{
Object *obj = OBJECT(objptr);
+ uint32_t ref;
+
if (!obj) {
return NULL;
}
- qatomic_inc(&obj->ref);
+ ref = qatomic_fetch_inc(&obj->ref);
+ /* Assert waaay before the integer overflows */
+ g_assert(ref < INT_MAX);
return obj;
}