On Mon, Oct 7, 2013 at 10:02 AM, Christophe Fergeau <cfergeau@xxxxxxxxxx> wrote:
> 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
I just sent another patch for that, which is a seperate issue.
> - 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
Perhaps slightly nicer, but also less flexible (you can't decide your
allocation scheme), and less unified (some functions would still need
to take an allocated object as arguments, while other would allocate).
> 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@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
--
Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel