Re: [Qemu-devel] [PATCH] vnc: deal with surface NULL pointers

[Christian Borntraeger]

On 03/08/2018 05:18 PM, Gerd Hoffmann wrote:
> Secondary displays in multihead setups are allowed to have a NULL
> DisplaySurface.  Typically user interfaces handle this by hiding the
> window which shows the display in question.
> 
> This isn't an option for vnc though because it simply hasn't a concept
> of windows or outputs.  So handle the situation by showing a placeholder
> DisplaySurface instead.  Also check in console_select whenever a surface
> is preset in the first place before requesting an update.
> 
> This fixes a segfault which can be triggered by switching to an unused
> display (via vtrl-alt-<nr>) in a multihead setup, for example using
> -device virtio-vga,max_outputs=2.
> 
> Cc: Christian Borntraeger <address@hidden>
> Signed-off-by: Gerd Hoffmann <address@hidden>
> ---

Tested-by: Christian Borntraeger <address@hidden>

I can see the new placeholder message and the crash is gone.


>  include/ui/console.h |  2 ++
>  ui/console.c         | 10 ++++++----
>  ui/vnc.c             | 10 ++++++++++
>  3 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/include/ui/console.h b/include/ui/console.h
> index aae9e44cb3..5fca9afcbc 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -260,6 +260,8 @@ DisplaySurface *qemu_create_displaysurface_guestmem(int 
> width, int height,
>                                                      pixman_format_code_t 
> format,
>                                                      int linesize,
>                                                      uint64_t addr);
> +DisplaySurface *qemu_create_message_surface(int w, int h,
> +                                            const char *msg);
>  PixelFormat qemu_default_pixelformat(int bpp);
> 
>  DisplaySurface *qemu_create_displaysurface(int width, int height);
> diff --git a/ui/console.c b/ui/console.c
> index 6ab4ff3baf..348610dd43 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1039,8 +1039,10 @@ void console_select(unsigned int index)
>                      dcl->ops->dpy_gfx_switch(dcl, s->surface);
>                  }
>              }
> -            dpy_gfx_update(s, 0, 0, surface_width(s->surface),
> -                           surface_height(s->surface));
> +            if (s->surface) {
> +                dpy_gfx_update(s, 0, 0, surface_width(s->surface),
> +                               surface_height(s->surface));
> +            }
>          }
>          if (ds->have_text) {
>              dpy_text_resize(s, s->width, s->height);
> @@ -1370,8 +1372,8 @@ DisplaySurface *qemu_create_displaysurface_guestmem(int 
> width, int height,
>      return surface;
>  }
> 
> -static DisplaySurface *qemu_create_message_surface(int w, int h,
> -                                                   const char *msg)
> +DisplaySurface *qemu_create_message_surface(int w, int h,
> +                                            const char *msg)
>  {
>      DisplaySurface *surface = qemu_create_displaysurface(w, h);
>      pixman_color_t bg = color_table_rgb[0][QEMU_COLOR_BLACK];
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 13c28cabb0..e164eb798c 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -746,9 +746,19 @@ static void vnc_update_server_surface(VncDisplay *vd)
>  static void vnc_dpy_switch(DisplayChangeListener *dcl,
>                             DisplaySurface *surface)
>  {
> +    static const char placeholder_msg[] =
> +        "Display output is not active.";
> +    static DisplaySurface *placeholder;
>      VncDisplay *vd = container_of(dcl, VncDisplay, dcl);
>      VncState *vs;
> 
> +    if (surface == NULL) {
> +        if (placeholder == NULL) {
> +            placeholder = qemu_create_message_surface(640, 480, 
> placeholder_msg);
> +        }
> +        surface = placeholder;
> +    }
> +
>      vnc_abort_display_jobs(vd);
>      vd->ds = surface;
>