Re: [Qemu-devel] [PULL 6/7] qxl: use guest_monitor_config for local renderer.

[Peter Maydell]

On 27 September 2018 at 08:12, Gerd Hoffmann <address@hidden> wrote:
> When processing monitor config from guest store head0 width and height
> for single-head configurations.  Use these when creating the
> DisplaySurface in the local renderer.
>
> This fixes a rendering issue with wayland.  Wayland rounds up the
> framebuffer width and height to a multiple of 64, so with odd
> resolutions (800x600 for example) the framebuffer is larger than the
> actual screen.  The monitor config has the actual screen size though.
>
> This fixes guest display for anything using the local renderer
> (non-spice UI, screendump monitor command).
>
> Signed-off-by: Gerd Hoffmann <address@hidden>
> Reviewed-by: Marc-André Lureau <address@hidden>
> Message-id: address@hidden

> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index 8e9135d9c6..747986478f 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -259,6 +259,8 @@ static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, 
> qxl_async_io async)
>
>  static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl, int replay)
>  {
> +    QXLMonitorsConfig *cfg;
> +
>      trace_qxl_spice_monitors_config(qxl->id);
>      if (replay) {
>          /*
> @@ -286,6 +288,16 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice 
> *qxl, int replay)
>                  (uintptr_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
>                                            QXL_IO_MONITORS_CONFIG_ASYNC));
>      }
> +
> +    cfg = qxl_phys2virt(qxl, qxl->guest_monitors_config, 
> MEMSLOT_GROUP_GUEST);
> +    if (cfg->count == 1) {
> +        qxl->guest_primary.resized = 1;
> +        qxl->guest_head0_width  = cfg->heads[0].width;
> +        qxl->guest_head0_height = cfg->heads[0].height;
> +    } else {
> +        qxl->guest_head0_width  = 0;
> +        qxl->guest_head0_height = 0;
> +    }

Hi. Coverity points out (CID 1395986) that we fail to check
the return value from qxl_phys2virt() here. It can return NULL,
in which case we'll segfault trying to dereference it.

thanks
-- PMM