[Spice-devel] [PATCH] prevent free setting same cursor in worker in red_set_cursor

[Christophe Fergeau]

On Fri, Aug 21, 2015 at 10:26:40AM +0100, Frediano Ziglio wrote:
> Currently caller don't call red_set_cursor with cursor in worker->cursor
> but in theory is possible.
> Doing so could cause the cursor to be freed and than reused as initially
> the reference counter is 1 so object is freed but then attempted to
> be used again.
> Incrementing the reference counter before trying to free the object solve
> the problem.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/red_worker.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/server/red_worker.c b/server/red_worker.c
> index d4abc3c..e70c008 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -4831,10 +4831,10 @@ static void red_release_cursor(RedWorker *worker, CursorItem *cursor)
>  
>  static void red_set_cursor(RedWorker *worker, CursorItem *cursor)
>  {
> +    ++cursor->refs;
>      if (worker->cursor) {
>          red_release_cursor(worker, worker->cursor);
>      }
> -    ++cursor->refs;
>      worker->cursor = cursor;
>  }


On second thought, I'd just add a g_return_if_fail(cursor !=
worker->cursor); at the very beginning of the function. This way, if
this situation ever happens, we'll get a runtime warning, and the
function will work as expected if I'm not mistaken.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150827/3f72f84c/attachment.sig>