Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
On Mon, 12 Nov 2012, Gerd Hoffmann wrote: On 11/10/12 00:45, Marek Vasut wrote: Dear Gerd Hoffmann, On 11/09/12 10:50, Peter Maydell wrote: On 9 November 2012 10:42, Anthony Liguori wrote: While the abstract discussion is fun, it never hurts to be defensive. I agree the root cause is vmware-vga but checking in vnc doesn't hurt. Defensive programming would suggest doing the clipping in the console.c layer. That sounds a reasonable plan to me (especially if we've hit similar problems multiple times in the past). Fully agree, I'll cook up a patch as I'm touching that anyway. Question is just whenever we'll go silently fixup stuff in console.c or use assert()s to enforce callers getting this correct. I'd tend to use assert() as vmware-vga passing bogous stuff there IMHO indicates there is a bug in vmware-vga. Or rather some revisions of the guest X driver. Though it's worth investigating it in the right place indeed. That too, but we must add a check to qemu nevertheless. We can't trust the guest to not pass in bogous data, be it intentionally or by mistake. vmware-vga must sanity check the guest input no matter what, but validating the guests input once should be enougth. For vmware_vga you could take this: http://patchwork.ozlabs.org/patch/197904/ or modify it/come up with a similar patch as needed. Regards, BALATON Zoltan
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
Hi, >> I'd go for clipping rather than asserting too (no crash) in all layers >> as a defensive approach (console.c/vnc.c). > > Won't that be an unnecessary slowdown? Thats why I tend to prefer assert for additional sanity checks. They help finding bugs, but can optionally be compiled out. But adding new asserts just before a release isn't the smartest move indeed, maybe clip now and turn the checks into asserts once 1.3 is out of the door. cheers, Gerd
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
On 11/10/12 00:45, Marek Vasut wrote: > Dear Gerd Hoffmann, > >> On 11/09/12 10:50, Peter Maydell wrote: >>> On 9 November 2012 10:42, Anthony Liguori wrote: While the abstract discussion is fun, it never hurts to be defensive. I agree the root cause is vmware-vga but checking in vnc doesn't hurt. >>> >>> Defensive programming would suggest doing the clipping in the >>> console.c layer. That sounds a reasonable plan to me (especially >>> if we've hit similar problems multiple times in the past). >> >> Fully agree, I'll cook up a patch as I'm touching that anyway. >> >> Question is just whenever we'll go silently fixup stuff in console.c or >> use assert()s to enforce callers getting this correct. I'd tend to use >> assert() as vmware-vga passing bogous stuff there IMHO indicates there >> is a bug in vmware-vga. > > Or rather some revisions of the guest X driver. Though it's worth > investigating > it in the right place indeed. That too, but we must add a check to qemu nevertheless. We can't trust the guest to not pass in bogous data, be it intentionally or by mistake. vmware-vga must sanity check the guest input no matter what, but validating the guests input once should be enougth. cheers, Gerd
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
Dear Gerhard Wiesinger, > On 10.11.2012 00:52, Peter Maydell wrote: > > On 10 November 2012 00:45, Marek Vasut wrote: > >> Gerd Hoffmann wrote: > >>> Question is just whenever we'll go silently fixup stuff in console.c or > >>> use assert()s to enforce callers getting this correct. I'd tend to use > >>> assert() as vmware-vga passing bogous stuff there IMHO indicates there > >>> is a bug in vmware-vga. > >> > >> Or rather some revisions of the guest X driver. > > > > If qemu's vmware-vga is blithely trusting what the guest driver > > hands it then that is itself a bug... > > > > To answer Gerd's question, I think I'd go for clip rather than assert > > (especially at this point in the release cycle), though I don't feel > > very strongly about it. > > I'd go for clipping rather than asserting too (no crash) in all layers > as a defensive approach (console.c/vnc.c). Won't that be an unnecessary slowdown? > Additionally logging that > condition would be helpful that the arising bug (which occurred several > times with a lot of unapplied fixes) can be detected by users easily and > fixed accordingly. Sounds good. > Ciao, > Gerhard Best regards, Marek Vasut
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
On Fri, Nov 9, 2012 at 7:13 AM, Gerhard Wiesinger wrote: > On 08.11.2012 22:07, Gerd Hoffmann wrote: >> >>Hi, >> >>> 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 ... >> > > Regarding fail safe programming I think it should be fixed/handled in both > modules: > vmware_vga.c should not trigger wrong values but also other modules should > verify or even correct there input parameters. > (think of situations where bits might not be accurate due to CPU bugs or > even QEMU/KVM in aerospace where > bits fall to other states due to high energy cosmic ray). > > Best solution is IHMO for vnc.c: > 1.) Log the problem (that other modules can be fixed, too). > 2.) Fix parameters (so that program doesn't crash) > > In mission critical software application like aerospace, airplanes, cars, > etc. (e.g. where people might get unhealthy) handling such situations where > input parameters aren't as expected is a must. There are plenty of other considerations. For example, QEMU tests don't have 100% code coverage, without even considering MC/DC. Adding two overlapping checks could even result in <100% coverage. Another issue is that the guest may cause QEMU to abort(). The code has not been formally reviewed, it's known to be in violation to the few specifications we have (HACKING, CODING_STYLE, docs/*). While QEMU is obviously a fine solution for many real world problems, I don't recommend using QEMU for any safety critical tasks. > See: > https://en.wikipedia.org/wiki/Fail-safe > https://en.wikipedia.org/wiki/Cosmic_ray#Effect_on_electronics > https://en.wikipedia.org/wiki/Radiation_hardening > > Precondition: > https://en.wikipedia.org/wiki/Eiffel_%28programming_language%29#Design_by_Contract > > Ciao, > Gerhard > >
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
On 10.11.2012 00:52, Peter Maydell wrote: On 10 November 2012 00:45, Marek Vasut wrote: Gerd Hoffmann wrote: Question is just whenever we'll go silently fixup stuff in console.c or use assert()s to enforce callers getting this correct. I'd tend to use assert() as vmware-vga passing bogous stuff there IMHO indicates there is a bug in vmware-vga. Or rather some revisions of the guest X driver. If qemu's vmware-vga is blithely trusting what the guest driver hands it then that is itself a bug... To answer Gerd's question, I think I'd go for clip rather than assert (especially at this point in the release cycle), though I don't feel very strongly about it. I'd go for clipping rather than asserting too (no crash) in all layers as a defensive approach (console.c/vnc.c). Additionally logging that condition would be helpful that the arising bug (which occurred several times with a lot of unapplied fixes) can be detected by users easily and fixed accordingly. Ciao, Gerhard
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
On 10 November 2012 00:45, Marek Vasut wrote: > Gerd Hoffmann wrote: >> Question is just whenever we'll go silently fixup stuff in console.c or >> use assert()s to enforce callers getting this correct. I'd tend to use >> assert() as vmware-vga passing bogous stuff there IMHO indicates there >> is a bug in vmware-vga. > > Or rather some revisions of the guest X driver. If qemu's vmware-vga is blithely trusting what the guest driver hands it then that is itself a bug... To answer Gerd's question, I think I'd go for clip rather than assert (especially at this point in the release cycle), though I don't feel very strongly about it. -- PMM
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
Dear Gerd Hoffmann, > On 11/09/12 10:50, Peter Maydell wrote: > > On 9 November 2012 10:42, Anthony Liguori wrote: > >> While the abstract discussion is fun, it never hurts to be defensive. I > >> agree the root cause is vmware-vga but checking in vnc doesn't hurt. > > > > Defensive programming would suggest doing the clipping in the > > console.c layer. That sounds a reasonable plan to me (especially > > if we've hit similar problems multiple times in the past). > > Fully agree, I'll cook up a patch as I'm touching that anyway. > > Question is just whenever we'll go silently fixup stuff in console.c or > use assert()s to enforce callers getting this correct. I'd tend to use > assert() as vmware-vga passing bogous stuff there IMHO indicates there > is a bug in vmware-vga. Or rather some revisions of the guest X driver. Though it's worth investigating it in the right place indeed. > cheers, > Gerd Best regards, Marek Vasut
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
On 11/09/12 10:50, Peter Maydell wrote: > On 9 November 2012 10:42, Anthony Liguori wrote: >> While the abstract discussion is fun, it never hurts to be defensive. I >> agree the root cause is vmware-vga but checking in vnc doesn't hurt. > > Defensive programming would suggest doing the clipping in the > console.c layer. That sounds a reasonable plan to me (especially > if we've hit similar problems multiple times in the past). Fully agree, I'll cook up a patch as I'm touching that anyway. Question is just whenever we'll go silently fixup stuff in console.c or use assert()s to enforce callers getting this correct. I'd tend to use assert() as vmware-vga passing bogous stuff there IMHO indicates there is a bug in vmware-vga. cheers, Gerd
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
On 9 November 2012 10:42, Anthony Liguori wrote: > While the abstract discussion is fun, it never hurts to be defensive. I > agree the root cause is vmware-vga but checking in vnc doesn't hurt. Defensive programming would suggest doing the clipping in the console.c layer. That sounds a reasonable plan to me (especially if we've hit similar problems multiple times in the past). -- PMM
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
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
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
Peter Maydell writes: > On 9 November 2012 08:13, Gerhard Wiesinger wrote: >> (think of situations where bits might not be accurate due to CPU bugs or >> even QEMU/KVM in aerospace where >> bits fall to other states due to high energy cosmic ray). > > If any aeroplane manufacturer is using QEMU for some safety critical > purpose it would be nice if they'd let us know. I could then avoid > flying with them in future :-) While the abstract discussion is fun, it never hurts to be defensive. I agree the root cause is vmware-vga but checking in vnc doesn't hurt. Regards, Anthony Liguori > > -- PMM
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
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
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
On 9 November 2012 08:13, Gerhard Wiesinger wrote: > (think of situations where bits might not be accurate due to CPU bugs or > even QEMU/KVM in aerospace where > bits fall to other states due to high energy cosmic ray). If any aeroplane manufacturer is using QEMU for some safety critical purpose it would be nice if they'd let us know. I could then avoid flying with them in future :-) -- PMM
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
On 08.11.2012 22:07, Gerd Hoffmann wrote: Hi, 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 ... Regarding fail safe programming I think it should be fixed/handled in both modules: vmware_vga.c should not trigger wrong values but also other modules should verify or even correct there input parameters. (think of situations where bits might not be accurate due to CPU bugs or even QEMU/KVM in aerospace where bits fall to other states due to high energy cosmic ray). Best solution is IHMO for vnc.c: 1.) Log the problem (that other modules can be fixed, too). 2.) Fix parameters (so that program doesn't crash) In mission critical software application like aerospace, airplanes, cars, etc. (e.g. where people might get unhealthy) handling such situations where input parameters aren't as expected is a must. See: https://en.wikipedia.org/wiki/Fail-safe https://en.wikipedia.org/wiki/Cosmic_ray#Effect_on_electronics https://en.wikipedia.org/wiki/Radiation_hardening Precondition: https://en.wikipedia.org/wiki/Eiffel_%28programming_language%29#Design_by_Contract Ciao, Gerhard
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
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? Regards, BALATON ZoltanFrom e1ea12f3fa70298f630c0b829d0f304339ca9799 Mon Sep 17 00:00:00 2001 From: BALATON Zoltan Date: Fri, 9 Nov 2012 00:44:29 +0100 Subject: [PATCH 2/2] vmware_vga: Clip updates with negative out of range rects to visible area Added checks and clipping also for negative out of range values in update rects which have been seen to happen at least with VNC under Windows NT 4.0 when a window is outside the visible area. Signed-off-by: BALATON Zoltan --- hw/vmware_vga.c | 16 1 file changed, 16 insertions(+) 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); -- 1.7.10
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
Hi, > 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 ... cheers, Gerd
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
On 1 November 2012 21:06, Gerhard Wiesinger wrote: > Fix crash with VNC under NT 4.0 and VMWare VGA and window which is outside > of the visible area. > > Backtrace: > #0 set_bit (addr=, nr=-3) at ./bitops.h:122 > #1 vnc_dpy_update (ds=, x=-48, y=145, w=57, h=161) at > ui/vnc.c:452 > #2 0x7f1ce057e2ec in dpy_update (s=0x7f1ce1c8c880, h=16, w=66, y=145, > x=-57) at ./console.h:242 > #3 vmsvga_update_rect (h=16, w=66, y=145, x=-57, s=0x7f1ce1cb3dd0) at > hw/vmware_vga.c:324 > #4 vmsvga_update_rect_flush (s=0x7f1ce1cb3dd0) at hw/vmware_vga.c:357 > #5 vmsvga_update_display (opaque=0x7f1ce1cb3dd0) at hw/vmware_vga.c:960 > #6 0x7f1ce05f0b37 in vnc_refresh (opaque=0x7f1cd8526010) at > ui/vnc.c:2590 > #7 0x7f1ce05c002b in qemu_run_timers (clock=0x7f1ce1c4f910) at > qemu-timer.c:392 > #8 qemu_run_timers (clock=0x7f1ce1c4f910) at qemu-timer.c:373 > #9 0x7f1ce05c028d in qemu_run_all_timers () at qemu-timer.c:449 > #10 0x7f1ce058f2ee in main_loop_wait (nonblocking=) at > main-loop.c:502 > #11 0x7f1ce047acb3 in main_loop () at vl.c:1655 > #12 main (argc=, argv=, envp=) > at vl.c:3826 > > Signed-off-by: Gerhard Wiesinger > --- > ui/vnc.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/ui/vnc.c b/ui/vnc.c > index 7c120e6..ae6d819 100644 > --- a/ui/vnc.c > +++ b/ui/vnc.c > @@ -453,6 +453,11 @@ static void vnc_dpy_update(DisplayState *ds, int x, int > y, int w, int h) > w = MIN(x + w, width) - x; > h = MIN(h, height); > > +x = MAX(x, 0); > +y = MAX(y, 0); > +w = MAX(w, 0); > +h = MAX(h, 0); > + > for (; y < h; y++) > for (i = 0; i < w; i += 16) > set_bit((x + i) / 16, s->dirty[y]); > -- > 1.7.11.7 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? -- PMM
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
Yet another ping. Ciao, Gerhard On 04.11.2012 11:28, Gerhard Wiesinger wrote: Ping? On 01.11.2012 21:06, Gerhard Wiesinger wrote: Fix crash with VNC under NT 4.0 and VMWare VGA and window which is outside of the visible area. Backtrace: #0 set_bit (addr=, nr=-3) at ./bitops.h:122 #1 vnc_dpy_update (ds=, x=-48, y=145, w=57, h=161) at ui/vnc.c:452 #2 0x7f1ce057e2ec in dpy_update (s=0x7f1ce1c8c880, h=16, w=66, y=145, x=-57) at ./console.h:242 #3 vmsvga_update_rect (h=16, w=66, y=145, x=-57, s=0x7f1ce1cb3dd0) at hw/vmware_vga.c:324 #4 vmsvga_update_rect_flush (s=0x7f1ce1cb3dd0) at hw/vmware_vga.c:357 #5 vmsvga_update_display (opaque=0x7f1ce1cb3dd0) at hw/vmware_vga.c:960 #6 0x7f1ce05f0b37 in vnc_refresh (opaque=0x7f1cd8526010) at ui/vnc.c:2590 #7 0x7f1ce05c002b in qemu_run_timers (clock=0x7f1ce1c4f910) at qemu-timer.c:392 #8 qemu_run_timers (clock=0x7f1ce1c4f910) at qemu-timer.c:373 #9 0x7f1ce05c028d in qemu_run_all_timers () at qemu-timer.c:449 #10 0x7f1ce058f2ee in main_loop_wait (nonblocking=out>) at main-loop.c:502 #11 0x7f1ce047acb3 in main_loop () at vl.c:1655 #12 main (argc=, argv=, envp=out>) at vl.c:3826 Signed-off-by: Gerhard Wiesinger --- ui/vnc.c | 5 + 1 file changed, 5 insertions(+) diff --git a/ui/vnc.c b/ui/vnc.c index 7c120e6..ae6d819 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -453,6 +453,11 @@ static void vnc_dpy_update(DisplayState *ds, int x, int y, int w, int h) w = MIN(x + w, width) - x; h = MIN(h, height); +x = MAX(x, 0); +y = MAX(y, 0); +w = MAX(w, 0); +h = MAX(h, 0); + for (; y < h; y++) for (i = 0; i < w; i += 16) set_bit((x + i) / 16, s->dirty[y]);
Re: [Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
Ping? On 01.11.2012 21:06, Gerhard Wiesinger wrote: Fix crash with VNC under NT 4.0 and VMWare VGA and window which is outside of the visible area. Backtrace: #0 set_bit (addr=, nr=-3) at ./bitops.h:122 #1 vnc_dpy_update (ds=, x=-48, y=145, w=57, h=161) at ui/vnc.c:452 #2 0x7f1ce057e2ec in dpy_update (s=0x7f1ce1c8c880, h=16, w=66, y=145, x=-57) at ./console.h:242 #3 vmsvga_update_rect (h=16, w=66, y=145, x=-57, s=0x7f1ce1cb3dd0) at hw/vmware_vga.c:324 #4 vmsvga_update_rect_flush (s=0x7f1ce1cb3dd0) at hw/vmware_vga.c:357 #5 vmsvga_update_display (opaque=0x7f1ce1cb3dd0) at hw/vmware_vga.c:960 #6 0x7f1ce05f0b37 in vnc_refresh (opaque=0x7f1cd8526010) at ui/vnc.c:2590 #7 0x7f1ce05c002b in qemu_run_timers (clock=0x7f1ce1c4f910) at qemu-timer.c:392 #8 qemu_run_timers (clock=0x7f1ce1c4f910) at qemu-timer.c:373 #9 0x7f1ce05c028d in qemu_run_all_timers () at qemu-timer.c:449 #10 0x7f1ce058f2ee in main_loop_wait (nonblocking=) at main-loop.c:502 #11 0x7f1ce047acb3 in main_loop () at vl.c:1655 #12 main (argc=, argv=, envp=out>) at vl.c:3826 Signed-off-by: Gerhard Wiesinger --- ui/vnc.c | 5 + 1 file changed, 5 insertions(+) diff --git a/ui/vnc.c b/ui/vnc.c index 7c120e6..ae6d819 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -453,6 +453,11 @@ static void vnc_dpy_update(DisplayState *ds, int x, int y, int w, int h) w = MIN(x + w, width) - x; h = MIN(h, height); +x = MAX(x, 0); +y = MAX(y, 0); +w = MAX(w, 0); +h = MAX(h, 0); + for (; y < h; y++) for (i = 0; i < w; i += 16) set_bit((x + i) / 16, s->dirty[y]);
[Qemu-devel] [PATCH] ui/vnc.c: Fix crash with VNC
Fix crash with VNC under NT 4.0 and VMWare VGA and window which is outside of the visible area. Backtrace: #0 set_bit (addr=, nr=-3) at ./bitops.h:122 #1 vnc_dpy_update (ds=, x=-48, y=145, w=57, h=161) at ui/vnc.c:452 #2 0x7f1ce057e2ec in dpy_update (s=0x7f1ce1c8c880, h=16, w=66, y=145, x=-57) at ./console.h:242 #3 vmsvga_update_rect (h=16, w=66, y=145, x=-57, s=0x7f1ce1cb3dd0) at hw/vmware_vga.c:324 #4 vmsvga_update_rect_flush (s=0x7f1ce1cb3dd0) at hw/vmware_vga.c:357 #5 vmsvga_update_display (opaque=0x7f1ce1cb3dd0) at hw/vmware_vga.c:960 #6 0x7f1ce05f0b37 in vnc_refresh (opaque=0x7f1cd8526010) at ui/vnc.c:2590 #7 0x7f1ce05c002b in qemu_run_timers (clock=0x7f1ce1c4f910) at qemu-timer.c:392 #8 qemu_run_timers (clock=0x7f1ce1c4f910) at qemu-timer.c:373 #9 0x7f1ce05c028d in qemu_run_all_timers () at qemu-timer.c:449 #10 0x7f1ce058f2ee in main_loop_wait (nonblocking=) at main-loop.c:502 #11 0x7f1ce047acb3 in main_loop () at vl.c:1655 #12 main (argc=, argv=, envp=) at vl.c:3826 Signed-off-by: Gerhard Wiesinger --- ui/vnc.c | 5 + 1 file changed, 5 insertions(+) diff --git a/ui/vnc.c b/ui/vnc.c index 7c120e6..ae6d819 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -453,6 +453,11 @@ static void vnc_dpy_update(DisplayState *ds, int x, int y, int w, int h) w = MIN(x + w, width) - x; h = MIN(h, height); +x = MAX(x, 0); +y = MAX(y, 0); +w = MAX(w, 0); +h = MAX(h, 0); + for (; y < h; y++) for (i = 0; i < w; i += 16) set_bit((x + i) / 16, s->dirty[y]); -- 1.7.11.7