[Spice-commits] server/red-worker.c

[Frediano Ziglio]

 server/red-worker.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

New commits:
commit 662d197a7dd65bdde17d6f47916362cbdc6774c3
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Fri Sep 1 16:04:21 2017 +0100

    red-worker: Fix leak processing update commands
    
    Is possible to have a leak processing update commands if
    the update command is synchronous and the rectangle list
    is empty. Note that Qemu always pass an empty list.
    
    If the list is empty display_channel_update fill the list.
    This is used to send back the list in case of asynchronous
    requests. But in handle_dev_update_async (the callback that
    handle the asynchronous case) the list is correctly freed.
    
    This was discovered by accident looking at the code.
    
    Reproduced with a Windows recording file using GCC address
    sanitizer and this patch to spice-server-replay:
    
      --- a/server/red-replay-qxl.c
      +++ b/server/red-replay-qxl.c
      @@ -1280,7 +1280,13 @@ static void replay_handle_dev_input(QXLWorker *worker, SpiceReplay *replay,
               replay->created_primary = FALSE;
               worker->destroy_surfaces(worker);
               break;
      -    case RED_WORKER_MESSAGE_UPDATE:
      +    case RED_WORKER_MESSAGE_UPDATE: {
      +        static uint8_t count = 0;
      +        QXLRect dummy;
      +        QXLRect update = { 0, 0, 100, 100 };
      +        count ^= 1;
      +        worker->update_area(worker, 0, &update, count ? &dummy : NULL, count ? 1 : 0, 0);
      +        } break;
               // XXX do anything? we record the correct bitmaps already.
           case RED_WORKER_MESSAGE_DISPLAY_CONNECT:
               // we want to ignore this one - it is sent on client connection, we
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/server/red-worker.c b/server/red-worker.c
index 4d8566be..675c232e 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -437,13 +437,17 @@ static void handle_dev_update(void *opaque, void *payload)
 {
     RedWorker *worker = opaque;
     RedWorkerMessageUpdate *msg = payload;
+    QXLRect *qxl_dirty_rects = msg->qxl_dirty_rects;
 
     spice_return_if_fail(worker->running);
 
     flush_display_commands(worker);
     display_channel_update(worker->display_channel,
                            msg->surface_id, msg->qxl_area, msg->clear_dirty_region,
-                           &msg->qxl_dirty_rects, &msg->num_dirty_rects);
+                           &qxl_dirty_rects, &msg->num_dirty_rects);
+    if (msg->qxl_dirty_rects == NULL) {
+        free(qxl_dirty_rects);
+    }
 }
 
 static void handle_dev_del_memslot(void *opaque, void *payload)