From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35787) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X1Fg7-00076b-DZ for qemu-devel@nongnu.org; Sun, 29 Jun 2014 10:02:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X1Fg2-0007ek-HU for qemu-devel@nongnu.org; Sun, 29 Jun 2014 10:01:59 -0400 Received: from mx-v6.kamp.de ([2a02:248:0:51::16]:37576 helo=mx01.kamp.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X1Fg2-0007eH-6I for qemu-devel@nongnu.org; Sun, 29 Jun 2014 10:01:54 -0400 Message-ID: <53B01C4E.6080107@kamp.de> Date: Sun, 29 Jun 2014 16:01:50 +0200 From: Peter Lieven MIME-Version: 1.0 References: <1403865671-16238-1-git-send-email-pl@kamp.de> In-Reply-To: <1403865671-16238-1-git-send-email-pl@kamp.de> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] ui/vnc: avoid memory corruption if width % VNC_DIRTY_PIXELS_PER_BIT != 0 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: kraxel@redhat.com, qemu-stable@nongnu.org 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 > --- > 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;