| Message ID | 20180308161803.6152-1-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | vnc: deal with surface NULL pointers | expand |
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 <borntraeger@de.ibm.com> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > 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; > >
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 <borntraeger@de.ibm.com> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> 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; >
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;
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 <borntraeger@de.ibm.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- include/ui/console.h | 2 ++ ui/console.c | 10 ++++++---- ui/vnc.c | 10 ++++++++++ 3 files changed, 18 insertions(+), 4 deletions(-)