The timer ID stored in ka->timer uses -1 as the sentinel value
meaning 'not initialized' (set in virKeepAliveNew). The checks
in virKeepAliveStop and virKeepAliveDispose used '> 0' or '>= 0'
to determine whether a timer is active.

However, if nexttimer overflows (see previous fix in vireventglib.c),
ka->timer can be a negative value other than -1 (e.g., -2147483648).
In that case:
- 'if (ka->timer > 0)' returns false -> virEventRemoveTimeout skipped
- 'if (ka->timer >= 0)' returns false -> virEventRemoveTimeout skipped

This leaves the GSource active after the keepalive object is freed,
causing a use-after-free with 'bad magic number' warnings in the log.

Fix by changing the checks to 'if (ka->timer != -1)', which correctly
handles both positive and negative timer IDs while still skipping
the sentinel value -1.

Additionally, add the same check in virKeepAliveDispose as a safety
net: if the object's refcount reaches 0 due to a race condition or
reference counting bug, the timer must be stopped before the object
is freed.

Signed-off-by: Yong Chen <[email protected]>
---
 src/rpc/virkeepalive.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/rpc/virkeepalive.c b/src/rpc/virkeepalive.c
index 690bc08b2e..4e7dd25d3c 100644
--- a/src/rpc/virkeepalive.c
+++ b/src/rpc/virkeepalive.c
@@ -218,6 +218,18 @@ virKeepAliveDispose(void *obj)
     PROBE(RPC_KEEPALIVE_DISPOSE,
           "ka=%p", ka);
 
+    /* Ensure the timer is stopped before freeing the object.
+     * This is a safety net: normally virKeepAliveStop is called
+     * before the object's refcount reaches 0, but in case of
+     * race conditions or reference counting bugs, we must not
+     * leave a dangling timer that would trigger use-after-free.
+     * Use != -1 (not >= 0) to also handle negative timer IDs
+     * caused by nexttimer integer overflow. */
+    if (ka->timer != -1) {
+        virEventRemoveTimeout(ka->timer);
+        ka->timer = -1;
+    }
+
     ka->freeCB(ka->client);
 }
 
@@ -301,7 +313,7 @@ virKeepAliveStop(virKeepAlive *ka)
           "ka=%p client=%p",
           ka, ka->client);
 
-    if (ka->timer > 0) {
+    if (ka->timer != -1) {
         virEventRemoveTimeout(ka->timer);
         ka->timer = -1;
     }
-- 
2.55.0.windows.3

Reply via email to