* [Qemu-devel] [PATCH] vnc: deal with surface NULL pointers
@ 2018-03-08 16:18 Gerd Hoffmann
2018-03-09 13:32 ` Philippe Mathieu-Daudé
2018-03-09 18:05 ` Christian Borntraeger
0 siblings, 2 replies; 3+ messages in thread
From: Gerd Hoffmann @ 2018-03-08 16:18 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Christian Borntraeger
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(-)
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;
--
2.9.3
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] vnc: deal with surface NULL pointers
2018-03-08 16:18 [Qemu-devel] [PATCH] vnc: deal with surface NULL pointers Gerd Hoffmann
@ 2018-03-09 13:32 ` Philippe Mathieu-Daudé
2018-03-09 18:05 ` Christian Borntraeger
1 sibling, 0 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-03-09 13:32 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel; +Cc: 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 <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;
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] vnc: deal with surface NULL pointers
2018-03-08 16:18 [Qemu-devel] [PATCH] vnc: deal with surface NULL pointers Gerd Hoffmann
2018-03-09 13:32 ` Philippe Mathieu-Daudé
@ 2018-03-09 18:05 ` Christian Borntraeger
1 sibling, 0 replies; 3+ messages in thread
From: Christian Borntraeger @ 2018-03-09 18:05 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel
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;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-03-09 18:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-08 16:18 [Qemu-devel] [PATCH] vnc: deal with surface NULL pointers Gerd Hoffmann
2018-03-09 13:32 ` Philippe Mathieu-Daudé
2018-03-09 18:05 ` Christian Borntraeger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).