[Spice-devel] [PATCH 4/6] worker: remove useless QXL_CMD_MESSAGE

[Frediano Ziglio]

> 
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
> 
> Unsecure code shouldn't be compiled in even in debug mode.
> Doesn't seem to be used, and probably should be handled at qemu qxl
> driver level instead.

Nack.

This would create leaks with old drivers as release is not handled.
Either should not compiled even on debug or should be checked for terminator
perhaps limiting possible string length.

Frediano

> ---
>  server/red_parse_qxl.c | 26 --------------------------
>  server/red_parse_qxl.h |  4 ----
>  server/red_worker.c    | 18 ------------------
>  3 files changed, 48 deletions(-)
> 
> diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
> index 5b1befa..7d5330c 100644
> --- a/server/red_parse_qxl.c
> +++ b/server/red_parse_qxl.c
> @@ -1176,32 +1176,6 @@ void red_put_update_cmd(RedUpdateCmd *red)
>      /* nothing yet */
>  }
>  
> -int red_get_message(RedMemSlotInfo *slots, int group_id,
> -                    RedMessage *red, QXLPHYSICAL addr)
> -{
> -    QXLMessage *qxl;
> -    int error;
> -
> -    /*
> -     * security alert:
> -     *   qxl->data[0] size isn't specified anywhere -> can't verify
> -     *   luckily this is for debug logging only,
> -     *   so we can just ignore it by default.
> -     */
> -    qxl = (QXLMessage *)get_virt(slots, addr, sizeof(*qxl), group_id,
> &error);
> -    if (error) {
> -        return 1;
> -    }
> -    red->release_info  = &qxl->release_info;
> -    red->data          = qxl->data;
> -    return 0;
> -}
> -
> -void red_put_message(RedMessage *red)
> -{
> -    /* nothing yet */
> -}
> -
>  int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
>                          RedSurfaceCmd *red, QXLPHYSICAL addr)
>  {
> diff --git a/server/red_parse_qxl.h b/server/red_parse_qxl.h
> index 3adc9fa..35fe72d 100644
> --- a/server/red_parse_qxl.h
> +++ b/server/red_parse_qxl.h
> @@ -124,10 +124,6 @@ int red_get_update_cmd(RedMemSlotInfo *slots, int
> group_id,
>                         RedUpdateCmd *red, QXLPHYSICAL addr);
>  void red_put_update_cmd(RedUpdateCmd *red);
>  
> -int red_get_message(RedMemSlotInfo *slots, int group_id,
> -                    RedMessage *red, QXLPHYSICAL addr);
> -void red_put_message(RedMessage *red);
> -
>  int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
>                          RedSurfaceCmd *red, QXLPHYSICAL addr);
>  void red_put_surface_cmd(RedSurfaceCmd *red);
> diff --git a/server/red_worker.c b/server/red_worker.c
> index d8a081e..38a41a2 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -5076,24 +5076,6 @@ static int red_process_commands(RedWorker *worker,
> uint32_t max_pipe_size, int *
>              red_put_update_cmd(&update);
>              break;
>          }
> -        case QXL_CMD_MESSAGE: {
> -            RedMessage message;
> -            QXLReleaseInfoExt release_info_ext;
> -
> -            if (red_get_message(&worker->mem_slots, ext_cmd.group_id,
> -                                &message, ext_cmd.cmd.data)) {
> -                break;
> -            }
> -#ifdef DEBUG
> -            /* alert: accessing message.data is insecure */
> -            spice_warning("MESSAGE: %s", message.data);
> -#endif
> -            release_info_ext.group_id = ext_cmd.group_id;
> -            release_info_ext.info = message.release_info;
> -            worker->qxl->st->qif->release_resource(worker->qxl,
> release_info_ext);
> -            red_put_message(&message);
> -            break;
> -        }
>          case QXL_CMD_SURFACE: {
>              RedSurfaceCmd *surface = spice_new0(RedSurfaceCmd, 1);
>  
> --
> 2.4.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>