[Spice-devel] [PATCH spice-server] red-worker: Fix leak processing update commands

[Frediano Ziglio]

> 
> On Mon, Sep 04, 2017 at 11:22:48AM -0400, Frediano Ziglio wrote:
> > > 
> > > Ah yeah, something might expect it was modified. Oh well, your initial
> > > suggestion is probably good then, though I'd explicitly check for a NULL
> > > msg->qxl_dirty_rects I think rather than msg->qxl_dirty_rects !=
> > > qxl_dirty_rects.
> > 
> > I cannot just check for NULL, Qemu could decide to pass a not NULL pointer,
> > in this can we can't free the memory used by Qemu without causing a
> > potential
> > invalid free (the pointer could be from stack or allocated with another
> > allocator). Actually I'm not 100% sure in all cases we get a NULL but
> > supposing NULL would be a API breakage.
> 
> The only case when you are going to allocate memory is if
> msg->qxl_dirty_rects is NULL.
> So you could do
> 
> if (msg->qxl_dirty_rects == NULL) {
>     free(qxl_dirty_rects);
> }

Just tried:

=================================================================
==15908==ERROR: AddressSanitizer: alloc-dealloc-mismatch (INVALID vs free) on 0x0ff17e9e7f20
    #0 0x7f8bffd784b8 in __interceptor_free (/lib64/libasan.so.4+0xde4b8)
    #1 0x7f8bff7817ff in handle_dev_update /home/freddy/work/spice-server/server/red-worker.c:450
    #2 0x7f8bff6cf38c in dispatcher_handle_single_read /home/freddy/work/spice-server/server/dispatcher.c:291
    #3 0x7f8bff6cf756 in dispatcher_handle_recv_read /home/freddy/work/spice-server/server/dispatcher.c:313
    #4 0x7f8bff7852a0 in handle_dev_input /home/freddy/work/spice-server/server/red-worker.c:1221
    #5 0x7f8bff6e3a57 in watch_func /home/freddy/work/spice-server/server/event-loop.c:128
    #6 0x7f8bfc80738c in g_main_dispatch /builddir/build/BUILD/glib-2.52.3/glib/gmain.c:3234
    #7 0x7f8bfc80738c in g_main_context_dispatch /builddir/build/BUILD/glib-2.52.3/glib/gmain.c:3899
    #8 0x7f8bfc807737 in g_main_context_iterate /builddir/build/BUILD/glib-2.52.3/glib/gmain.c:3972
    #9 0x7f8bfc807a81 in g_main_loop_run /builddir/build/BUILD/glib-2.52.3/glib/gmain.c:4168
    #10 0x7f8bff78661b in red_worker_main /home/freddy/work/spice-server/server/red-worker.c:1381
    #11 0x7f8bfbe6f36c in start_thread (/lib64/libpthread.so.0+0x736c)
    #12 0x7f8bfbba7bbe in __GI___clone (/lib64/libc.so.6+0x110bbe)

0x7f8bf4f40690 is located 140239262567288 bytes to the left of 140239268765951-byte region [0x60600002e750,0xdfebffcf484f)
ASAN:DEADLYSIGNAL
AddressSanitizer: nested bug in the same thread, aborting.


> rather than
> 
> if (msg->qxl_dirty_rects != qxl_dirty_rects) {
>     free(qxl_dirty_rects);
> }
> 

Frediano