On Thu, Jul 23, 2026 at 07:47:56PM +0800, Yong Chen via Devel wrote: > 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.
That's a surprisingly high usage of timers. IIUC that works out at the daemon using 7,000,000 per day, or 5000 per minute. What workload are you imposing on the daemon to hit that kind of incredibly high rate of timer ID increments ? > > 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; If we need to guard against this, then we need to stop using a simple integer counter, and instead track a list of ranges of unused timer IDs. A fair bit more complex, but it would allow us to have the full set of 2^32 concurrent timers active. Initially we would have a free range start=0 end=4294967296 After some time allocating timers, this ends up with a free range start=10 end=4294967296 if we then free 4 timers in the middle, we might get two ranges start=4 end=7 -- start=10 end=4294967296 and so on, as we free timers, we either decrement the 'start' of an existing range, or have to add a new range to the list. Always allocating from the lowest range would stop us getting too many new ranges over time. > data->interval = interval; > data->cb = cb; > data->opaque = opaque; > -- > 2.55.0.windows.3 > With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
