On Thu, Mar 11, 2021 at 10:38 PM Daniel P. Berrangé <berra...@redhat.com>
wrote:
> A long time ago the VNC server code had some memory corruption
> fixes done in:
>
> commit bea60dd7679364493a0d7f5b54316c767cf894ef
> Author: Peter Lieven <p...@kamp.de>
> Date: Mon Jun 30 10:57:51 2014 +0200
>
> ui/vnc: fix potential memory corruption issues
>
> One of the implications of the fix was that the VNC server would have a
> thin black bad down the right hand side if the guest desktop width was
> not a multiple of 16. In practice this was a non-issue since the VNC
> server was always honouring a guest specified resolution and guests
> essentially always pick from a small set of sane resolutions likely in
> real world hardware.
>
> We recently introduced support for the extended desktop resize extension
> and as a result the VNC client has ability to specify an arbitrary
> desktop size and the guest OS may well honour it exactly. As a result we
> no longer have any guarantee that the width will be a multiple of 16,
> and so when resizing the desktop we have a 93% chance of getting the
> black bar on the right hand size.
>
> The VNC server maintains three different desktop dimensions
>
> 1. The guest surface
> 2. The server surface
> 3. The client desktop
>
> The requirement for the width to be a multiple of 16 only applies to
> item 2, the server surface, for the purpose of doing dirty bitmap
> tracking.
>
> Normally we will set the client desktop size to always match the server
> surface size, but that's not a strict requirement. In order to cope with
> clients that don't support the desktop size encoding, we already allow
> for the client desktop to be a different size that the server surface.
>
> Thus we can trivially eliminate the black bar, but setting the client
> desktop size to be the un-rounded server surface size - the so called
> "true width".
>
> Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
> ---
> ui/trace-events | 2 ++
> ui/vnc.c | 23 +++++++++++++++++++----
> ui/vnc.h | 1 +
> 3 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/ui/trace-events b/ui/trace-events
> index 3838ae2d84..5d1da6f236 100644
> --- a/ui/trace-events
> +++ b/ui/trace-events
> @@ -59,6 +59,8 @@ vnc_client_throttle_audio(void *state, void *ioc, size_t
> offset) "VNC client thr
> vnc_client_unthrottle_forced(void *state, void *ioc) "VNC client
> unthrottle forced offset state=%p ioc=%p"
> vnc_client_unthrottle_incremental(void *state, void *ioc, size_t offset)
> "VNC client unthrottle incremental state=%p ioc=%p offset=%zu"
> vnc_client_output_limit(void *state, void *ioc, size_t offset, size_t
> threshold) "VNC client output limit state=%p ioc=%p offset=%zu
> threshold=%zu"
> +vnc_server_dpy_pageflip(void *dpy, int w, int h, int fmt) "VNC server dpy
> pageflip dpy=%p size=%dx%d fmt=%d"
> +vnc_server_dpy_recreate(void *dpy, int w, int h, int fmt) "VNC server dpy
> recreate dpy=%p size=%dx%d fmt=%d"
> vnc_job_add_rect(void *state, void *job, int x, int y, int w, int h) "VNC
> add rect state=%p job=%p offset=%d,%d size=%dx%d"
> vnc_job_discard_rect(void *state, void *job, int x, int y, int w, int h)
> "VNC job discard rect state=%p job=%p offset=%d,%d size=%dx%d"
> vnc_job_clamp_rect(void *state, void *job, int x, int y, int w, int h)
> "VNC job clamp rect state=%p job=%p offset=%d,%d size=%dx%d"
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 8c9890b3cd..9c004a11f4 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -608,6 +608,11 @@ static int vnc_width(VncDisplay *vd)
> VNC_DIRTY_PIXELS_PER_BIT));
> }
>
> +static int vnc_true_width(VncDisplay *vd)
> +{
> + return MIN(VNC_MAX_WIDTH, surface_width(vd->ds));
> +}
> +
> static int vnc_height(VncDisplay *vd)
> {
> return MIN(VNC_MAX_HEIGHT, surface_height(vd->ds));
> @@ -691,16 +696,16 @@ static void vnc_desktop_resize(VncState *vs)
> !vnc_has_feature(vs,
> VNC_FEATURE_RESIZE_EXT))) {
> return;
> }
> - if (vs->client_width == pixman_image_get_width(vs->vd->server) &&
> + if (vs->client_width == vs->vd->true_width &&
> vs->client_height == pixman_image_get_height(vs->vd->server)) {
> return;
> }
>
> - assert(pixman_image_get_width(vs->vd->server) < 65536 &&
> - pixman_image_get_width(vs->vd->server) >= 0);
> + assert(vs->vd->true_width < 65536 &&
> + vs->vd->true_width >= 0);
> assert(pixman_image_get_height(vs->vd->server) < 65536 &&
> pixman_image_get_height(vs->vd->server) >= 0);
> - vs->client_width = pixman_image_get_width(vs->vd->server);
> + vs->client_width = vs->vd->true_width;
> vs->client_height = pixman_image_get_height(vs->vd->server);
>
> if (vnc_has_feature(vs, VNC_FEATURE_RESIZE_EXT)) {
> @@ -774,6 +779,7 @@ static void vnc_update_server_surface(VncDisplay *vd)
>
> width = vnc_width(vd);
> height = vnc_height(vd);
> + vd->true_width = vnc_true_width(vd);
> vd->server = pixman_image_create_bits(VNC_SERVER_FB_FORMAT,
> width, height,
> NULL, 0);
> @@ -809,13 +815,22 @@ static void vnc_dpy_switch(DisplayChangeListener
> *dcl,
> vd->guest.fb = pixman_image_ref(surface->image);
> vd->guest.format = surface->format;
>
> +
> if (pageflip) {
> + trace_vnc_server_dpy_pageflip(vd,
> + surface_width(surface),
> + surface_height(surface),
> + surface_format(surface));
> vnc_set_area_dirty(vd->guest.dirty, vd, 0, 0,
> surface_width(surface),
> surface_height(surface));
> return;
> }
>
> + trace_vnc_server_dpy_recreate(vd,
> + surface_width(surface),
> + surface_height(surface),
> + surface_format(surface));
> /* server surface */
> vnc_update_server_surface(vd);
>
> diff --git a/ui/vnc.h b/ui/vnc.h
> index 116463d5f0..d4f3e15558 100644
> --- a/ui/vnc.h
> +++ b/ui/vnc.h
> @@ -164,6 +164,7 @@ struct VncDisplay
>
> struct VncSurface guest; /* guest visible surface (aka ds->surface)
> */
> pixman_image_t *server; /* vnc server surface */
> + int true_width; /* server surface width before rounding up */
>
> const char *id;
> QTAILQ_ENTRY(VncDisplay) next;
> --
> 2.29.2
>
>
>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
--
Marc-André Lureau
