[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);
> }
> rather than
> 
> if (msg->qxl_dirty_rects != qxl_dirty_rects) {
>     free(qxl_dirty_rects);
> }
> 

If msg->qxl_dirty_rects was not NULL if passed from the dispatcher
display_channel_update do not allocate memory and do not change the
pointer which came from Qemu so you are going to free that pointer.
msg->qxl_dirty_rects will never be NULL as it is filled if was NULL
(at least with current code).

Frediano