Message ID | alpine.GSO.2.00.1211090054080.11771@mono |
---|---|
State | New |
Headers | show |
On 09.11.2012 03:55, BALATON Zoltan wrote: > On Thu, 8 Nov 2012, Gerd Hoffmann wrote: >>> I think this is fixing this at the wrong level. Either we >>> should require that drivers (in this case vmware_vga.c) >>> must not call dpy_gfx_update() with out of range values, >>> or we should do the clipping in the console.c layer, but >>> I don't think requiring every UI backend to clip is the >>> right thing. Anthony? >> >> Agree. IMHO vmware_vga.c is at fault here and should be fixed. We can >> add some asserts to console.[ch] to enforce this ... > > Would the attached patch help? I fixed this 2 times, and I remember two other people fixing the same thing too already. Lemme find some refs... thread.gmane.org/gmane.comp.emulators.qemu/166064 --- Is it the same as https://bugs.launchpad.net/bugs/918791 ? At least it appears to be the same theme... But there, the patch (https://launchpadlibrarian.net/94916786/qemu-vmware.debdiff) also updates width/height. My comment: https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791/comments/21 --- Adding some Cc's....
On 09.11.2012 13:00, Michael Tokarev wrote: > On 09.11.2012 03:55, BALATON Zoltan wrote: >> On Thu, 8 Nov 2012, Gerd Hoffmann wrote: >>>> I think this is fixing this at the wrong level. Either we >>>> should require that drivers (in this case vmware_vga.c) >>>> must not call dpy_gfx_update() with out of range values, >>>> or we should do the clipping in the console.c layer, but >>>> I don't think requiring every UI backend to clip is the >>>> right thing. Anthony? >>> >>> Agree. IMHO vmware_vga.c is at fault here and should be fixed. We can >>> add some asserts to console.[ch] to enforce this ... >> >> Would the attached patch help? > > I fixed this 2 times, and I remember two other people fixing > the same thing too already. Lemme find some refs... > > http://thread.gmane.org/gmane.comp.emulators.qemu/166064 > > --- > Is it the same as https://bugs.launchpad.net/bugs/918791 ? > At least it appears to be the same theme... But there, > the patch (https://launchpadlibrarian.net/94916786/qemu-vmware.debdiff) > also updates width/height. My comment: > https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791/comments/21 > --- Another reference: the same problem in qxl (Gerd should know this area): http://thread.gmane.org/gmane.comp.emulators.qemu/171093 this patch is a cleanup, -- the problem has been fixed twice in a row in qxl. We've 3 fixes for it in vmware now too. So figuring out the proper level where to fix it is important... /mjt
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c index 834588d..e59ab3a 100644 --- a/hw/vmware_vga.c +++ b/hw/vmware_vga.c @@ -296,6 +296,14 @@ static inline void vmsvga_update_rect(struct vmsvga_state_s *s, uint8_t *src; uint8_t *dst; + if (x < 0 || x + w < 0) { + fprintf(stderr, "%s: update negative x position: %d, w: %d\n", + __func__, x, w); + w -= x; + x = MAX(x, 0); + y = MAX(w, 0); + } + if (x + w > ds_get_width(s->vga.ds)) { fprintf(stderr, "%s: update width too large x: %d, w: %d\n", __func__, x, w); @@ -303,6 +311,14 @@ static inline void vmsvga_update_rect(struct vmsvga_state_s *s, w = ds_get_width(s->vga.ds) - x; } + if (y < 0 || y + h < 0) { + fprintf(stderr, "%s: update negative y position: %d, h: %d\n", + __func__, y, h); + h -= y; + y = MAX(y, 0); + h = MAX(h, 0); + } + if (y + h > ds_get_height(s->vga.ds)) { fprintf(stderr, "%s: update height too large y: %d, h: %d\n", __func__, y, h);