The 'nexttimer' variable is a static int that starts at 1 and is
incremented every time a new timeout is registered via
virEventGLibTimeoutAdd. After running for a long period of time
(many months/years with frequent client connections), nexttimer
can overflow past INT_MAX and become negative.

A negative timer ID causes multiple problems:
1. virKeepAliveStart: 'if (ka->timer < 0)' returns true, skipping
   virObjectRef() which is needed to hold the timer's reference to
   the keepalive object. This leads to premature object freeing.
2. virKeepAliveStop: 'if (ka->timer > 0)' returns false, skipping
   virEventRemoveTimeout(). The GSource is never destroyed, causing
   a use-after-free when the timer fires on the freed object.
3. virNetServerClientNew: 'if (client->sockTimer < 0)' returns true,
   causing client creation to fail with 'Connection reset by peer'.

In a production environment running for 1 year and 8 months,
nexttimer overflowed and accumulated 282 million leaked timer
entries, consuming 19.6GB of memory.

Fix by resetting nexttimer to 1 when it overflows below 1, ensuring
timer IDs are always positive.

Signed-off-by: Yong Chen <[email protected]>
---
 src/util/vireventglib.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/util/vireventglib.c b/src/util/vireventglib.c
index 6c54f62123..6fe23b3b7d 100644
--- a/src/util/vireventglib.c
+++ b/src/util/vireventglib.c
@@ -341,6 +341,9 @@ virEventGLibTimeoutAdd(int interval,
 
     data = g_new0(struct virEventGLibTimeout, 1);
     data->timer = nexttimer++;
+    /* Guard against integer overflow: timer IDs must be positive */
+    if (nexttimer < 1)
+        nexttimer = 1;
     data->interval = interval;
     data->cb = cb;
     data->opaque = opaque;
-- 
2.55.0.windows.3

Reply via email to