[Spice-devel] [PATCH] RFC: server: plug some leaks on error

[Christophe Fergeau]

Hey,

Patch looks good, a few comments:
- handle_dev_loadvm_commands() would have the same surface_cmd leak
  except that it does not check red_get_surface_cmd() return value,
  probably a bug as well
- things would be slightly nicer if
  red_get_surface_cmd()/red_get_cursor_cmd() returned a newly allocated
  RedCursorCmd/RedSurfaceCmd rather than the caller having to allocate it
  right before the call

Christophe

On Fri, Oct 04, 2013 at 08:59:55PM +0200, Marc-André Lureau wrote:
> Plug what looks like memory leaks, that could be potentially be
> triggered by a misbehaving guest.
> ---
>  server/red_worker.c | 9 +++++++--
>  spice-common        | 2 +-
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/server/red_worker.c b/server/red_worker.c
> index f5a5553..8f7a1fc 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -4949,10 +4949,13 @@ static int red_process_cursor(RedWorker *worker, uint32_t max_pipe_size, int *ri
>          case QXL_CMD_CURSOR: {
>              RedCursorCmd *cursor = spice_new0(RedCursorCmd, 1);
>  
> -            if (!red_get_cursor_cmd(&worker->mem_slots, ext_cmd.group_id,
> +            if (red_get_cursor_cmd(&worker->mem_slots, ext_cmd.group_id,
>                                      cursor, ext_cmd.cmd.data)) {
> -                qxl_process_cursor(worker, cursor, ext_cmd.group_id);
> +                free(cursor);
> +                break;
>              }
> +
> +            qxl_process_cursor(worker, cursor, ext_cmd.group_id);
>              break;
>          }
>          default:
> @@ -5058,6 +5061,7 @@ static int red_process_commands(RedWorker *worker, uint32_t max_pipe_size, int *
>  
>              if (red_get_surface_cmd(&worker->mem_slots, ext_cmd.group_id,
>                                      surface, ext_cmd.cmd.data)) {
> +                free(surface);
>                  break;
>              }
>              red_process_surface(worker, surface, ext_cmd.group_id, FALSE);
> @@ -11647,6 +11651,7 @@ void handle_dev_loadvm_commands(void *opaque, void *payload)
>                  /* XXX allow failure in loadvm? */
>                  spice_warning("failed loadvm command type (%d)",
>                                ext[i].cmd.type);
> +                free(cursor_cmd);
>                  continue;
>              }
>              qxl_process_cursor(worker, cursor_cmd, ext[i].group_id);
> diff --git a/spice-common b/spice-common
> index e443c9f..7e8ba10 160000
> --- a/spice-common
> +++ b/spice-common
> @@ -1 +1 @@
> -Subproject commit e443c9f6039407633d38a0eba03c344272ac8559
> +Subproject commit 7e8ba10779a3fb11d587e8a59fe389acd2412dd0
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20131007/be6754e8/attachment.pgp>