* [Qemu-devel] [PATCH] ui/vnc: avoid memory corruption if width % VNC_DIRTY_PIXELS_PER_BIT != 0
@ 2014-06-27 10:41 Peter Lieven
2014-06-29 14:01 ` Peter Lieven
0 siblings, 1 reply; 2+ messages in thread
From: Peter Lieven @ 2014-06-27 10:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Lieven, kraxel, qemu-stable
during resolution change in Windows 7 it happens sometimes that Windows changes to
an intermediate resolution where server_stride % cmp_bytes != 0 (in vnc_refresh_server_surface).
The problem that causes memory corruption is where the guest fb is copied to the server fb.
It could be easily fixed by truncating cmp_bytes in vnc_refresh_server_surface. But by looking at
the code it seems that none of the encoders called in vnc_send_framebuffer_update really cares about
w > pixman_image_get_width(vd->server). This patch will therefore remove all DIV_ROUND_UPs for
now to avoid corruption or illegal reads. I think there are really almost no real resultions out
there where width % 16 != 0. If we really find some we might need to either decrease
VNC_DIRTY_PIXELS_PER_BIT or make it dynamic depending on the resolution.
Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Lieven <pl@kamp.de>
---
ui/vnc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/ui/vnc.c b/ui/vnc.c
index 14a86c3..9e37d47 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -577,7 +577,7 @@ void *vnc_server_fb_ptr(VncDisplay *vd, int x, int y)
memset(bitmap, 0x00, sizeof(bitmap));\
for (y = 0; y < h; y++) {\
bitmap_set(bitmap[y], 0,\
- DIV_ROUND_UP(w, VNC_DIRTY_PIXELS_PER_BIT));\
+ w / VNC_DIRTY_PIXELS_PER_BIT);\
} \
}
@@ -2738,7 +2738,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
}
guest_ptr += x * cmp_bytes;
- for (; x < DIV_ROUND_UP(width, VNC_DIRTY_PIXELS_PER_BIT);
+ for (; x < width / VNC_DIRTY_PIXELS_PER_BIT;
x++, guest_ptr += cmp_bytes, server_ptr += cmp_bytes) {
if (!test_and_clear_bit(x, vd->guest.dirty[y])) {
continue;
--
1.7.9.5
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH] ui/vnc: avoid memory corruption if width % VNC_DIRTY_PIXELS_PER_BIT != 0
2014-06-27 10:41 [Qemu-devel] [PATCH] ui/vnc: avoid memory corruption if width % VNC_DIRTY_PIXELS_PER_BIT != 0 Peter Lieven
@ 2014-06-29 14:01 ` Peter Lieven
0 siblings, 0 replies; 2+ messages in thread
From: Peter Lieven @ 2014-06-29 14:01 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, qemu-stable
If you find that patch too strict, I have another patch ready (needs some final testing) which
works around all the possible corruption issues iff
a) width % VNC_DIRTY_PIXELS_PER_BIT != 0 (while still keep it working)
b) width > VNC_MAX_WIDTH || heigth > VNC_MAX_HEIGTH
Peter
Am 27.06.2014 12:41, schrieb Peter Lieven:
> during resolution change in Windows 7 it happens sometimes that Windows changes to
> an intermediate resolution where server_stride % cmp_bytes != 0 (in vnc_refresh_server_surface).
> The problem that causes memory corruption is where the guest fb is copied to the server fb.
> It could be easily fixed by truncating cmp_bytes in vnc_refresh_server_surface. But by looking at
> the code it seems that none of the encoders called in vnc_send_framebuffer_update really cares about
> w > pixman_image_get_width(vd->server). This patch will therefore remove all DIV_ROUND_UPs for
> now to avoid corruption or illegal reads. I think there are really almost no real resultions out
> there where width % 16 != 0. If we really find some we might need to either decrease
> VNC_DIRTY_PIXELS_PER_BIT or make it dynamic depending on the resolution.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Peter Lieven <pl@kamp.de>
> ---
> ui/vnc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 14a86c3..9e37d47 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -577,7 +577,7 @@ void *vnc_server_fb_ptr(VncDisplay *vd, int x, int y)
> memset(bitmap, 0x00, sizeof(bitmap));\
> for (y = 0; y < h; y++) {\
> bitmap_set(bitmap[y], 0,\
> - DIV_ROUND_UP(w, VNC_DIRTY_PIXELS_PER_BIT));\
> + w / VNC_DIRTY_PIXELS_PER_BIT);\
> } \
> }
>
> @@ -2738,7 +2738,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
> }
> guest_ptr += x * cmp_bytes;
>
> - for (; x < DIV_ROUND_UP(width, VNC_DIRTY_PIXELS_PER_BIT);
> + for (; x < width / VNC_DIRTY_PIXELS_PER_BIT;
> x++, guest_ptr += cmp_bytes, server_ptr += cmp_bytes) {
> if (!test_and_clear_bit(x, vd->guest.dirty[y])) {
> continue;
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-06-29 14:02 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-27 10:41 [Qemu-devel] [PATCH] ui/vnc: avoid memory corruption if width % VNC_DIRTY_PIXELS_PER_BIT != 0 Peter Lieven
2014-06-29 14:01 ` Peter Lieven
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.