@samitouri / QOSamiQemu / commits / 3cae0b46be

ui/vnc-jobs: fix VncRectEntry leak on job cleanup

When a VncJob is freed, its associated VncRectEntry list must also be freed. Previously, vnc_job_push() and the disconnected path in vnc_worker_thread_loop() called g_free(job) directly, leaking all VncRectEntry allocations. Introduce vnc_job_free() which iterates and frees the rectangle entries before freeing the job itself, and use it in both paths. Also add QLIST_REMOVE() in the worker loop before g_free(entry), so that entries processed during normal operation are properly unlinked. Without this, vnc_job_free() would iterate dangling pointers to already-freed entries, causing use-after-free. Fixes: bd023f953e5e ("vnc: threaded VNC server") Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Marc-André Lureau committed Mar 13, 2026 at 20:54 UTC 3cae0b46be5416b26039df5259ffc8fcf2989516
1 file changed +17 -2
ui/vnc-jobs.c
+17 -2
@@ -107,11 +107,25 @@ int vnc_job_add_rect(VncJob *job, int x, int y, int w, int h)
107 return 1;
108 }
109
110 +static void vnc_job_free(VncJob *job)
111 +{
112 + VncRectEntry *entry, *tmp;
113 +
114 + if (!job) {
115 + return;
116 + }
117 + QLIST_FOREACH_SAFE(entry, &job->rectangles, next, tmp) {
118 + /* no need for QLIST_REMOVE(entry, next) */
119 + g_free(entry);
120 + }
121 + g_free(job);
122 +}
123 +
124 void vnc_job_push(VncJob *job)
125 {
126 vnc_lock_queue(queue);
127 if (queue->exit || QLIST_EMPTY(&job->rectangles)) {
114 - g_free(job);
128 + vnc_job_free(job);
129 } else {
130 QTAILQ_INSERT_TAIL(&queue->jobs, job, next);
131 qemu_cond_broadcast(&queue->cond);
@@ -296,6 +310,7 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
310 n_rectangles += n;
311 }
312 }
313 + QLIST_REMOVE(entry, next);
314 g_free(entry);
315 }
316 trace_vnc_job_nrects(&vs, job, n_rectangles);
@@ -324,7 +339,7 @@ disconnected:
339 QTAILQ_REMOVE(&queue->jobs, job, next);
340 vnc_unlock_queue(queue);
341 qemu_cond_broadcast(&queue->cond);
327 - g_free(job);
342 + vnc_job_free(job);
343 vs.magic = 0;
344 return 0;
345 }