59 lines
1.9 KiB
Diff
59 lines
1.9 KiB
Diff
|
From 7d94ca45a9c0ef8a4f1917f41496a826ecda90fb Mon Sep 17 00:00:00 2001
|
||
|
|
From: "Michael S. Tsirkin" <mst@redhat.com>
|
||
|
|
Date: Fri, 25 Feb 2022 08:40:27 -0500
|
||
|
|
Subject: [PATCH 3/6] 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>
|
||
|
|
Signed-off-by: wanbo <wanbo13@huawei.com>
|
||
|
|
---
|
||
|
|
qom/object.c | 6 +++++-
|
||
|
|
1 file changed, 5 insertions(+), 1 deletion(-)
|
||
|
|
|
||
|
|
diff --git a/qom/object.c b/qom/object.c
|
||
|
|
index 4f0677cca9..5db3974f04 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;
|
||
|
|
}
|
||
|
|
|
||
|
|
--
|
||
|
|
2.27.0
|
||
|
|
|