From e36828787f2c35df64448f4142169ef5e193f5b3 Mon Sep 17 00:00:00 2001 From: Michael S. Tsirkin Date: Fri, 25 Feb 2022 08:40:27 -0500 Subject: 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 --- qom/object.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'qom') 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; } -- cgit v1.2.3-55-g7522