* [Qemu-devel] [RFC] fix crashes with vmware driver
@ 2012-03-02 21:11 Serge Hallyn
2012-03-02 21:20 ` Ryan Harper
0 siblings, 1 reply; 5+ messages in thread
From: Serge Hallyn @ 2012-03-02 21:11 UTC (permalink / raw)
To: qemu-devel
Hi,
I don't know where the best place to catch this would be, but
with vnc and vmware_vga it's possible to get set_bit called on
a negative index, crashing qemu. See
https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791
for details. This patch prevents that. It's possible this
should be caught earlier, but this patch works for me.
Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com>
---
vmware_vga.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
Index: qemu-kvm-1.0+noroms/hw/vmware_vga.c
===================================================================
--- qemu-kvm-1.0+noroms.orig/hw/vmware_vga.c 2012-03-01 16:19:23.280571798 -0600
+++ qemu-kvm-1.0+noroms/hw/vmware_vga.c 2012-03-01 16:27:27.910975006 -0600
@@ -298,6 +298,22 @@
uint8_t *src;
uint8_t *dst;
+ if (x < 0) {
+ fprintf(stderr, "%s: update x was < 0 (%d, w %d)\n",
+ __FUNCTION__, x, w);
+ w += x;
+ if (w < 0)
+ return;
+ x = 0;
+ }
+ if (y < 0) {
+ fprintf(stderr, "%s: update y was < 0 (%d, h %d)\n",
+ __FUNCTION__, y, h);
+ h += y;
+ if (h < 0)
+ return;
+ y = 0;
+ }
if (x + w > s->width) {
fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
__FUNCTION__, x, w);
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [RFC] fix crashes with vmware driver
2012-03-02 21:11 [Qemu-devel] [RFC] fix crashes with vmware driver Serge Hallyn
@ 2012-03-02 21:20 ` Ryan Harper
2012-03-05 19:33 ` Serge E. Hallyn
2012-03-05 19:33 ` [Qemu-devel] [PATCH 1/1] vmware_vga: stop crashing Serge Hallyn
0 siblings, 2 replies; 5+ messages in thread
From: Ryan Harper @ 2012-03-02 21:20 UTC (permalink / raw)
To: Serge Hallyn; +Cc: qemu-devel
* Serge Hallyn <serge.hallyn@canonical.com> [2012-03-02 15:13]:
> Hi,
>
> I don't know where the best place to catch this would be, but
> with vnc and vmware_vga it's possible to get set_bit called on
> a negative index, crashing qemu. See
>
> https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791
>
> for details. This patch prevents that. It's possible this
> should be caught earlier, but this patch works for me.
>
> Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com>
> ---
> vmware_vga.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> Index: qemu-kvm-1.0+noroms/hw/vmware_vga.c
> ===================================================================
> --- qemu-kvm-1.0+noroms.orig/hw/vmware_vga.c 2012-03-01 16:19:23.280571798 -0600
> +++ qemu-kvm-1.0+noroms/hw/vmware_vga.c 2012-03-01 16:27:27.910975006 -0600
> @@ -298,6 +298,22 @@
> uint8_t *src;
> uint8_t *dst;
>
> + if (x < 0) {
> + fprintf(stderr, "%s: update x was < 0 (%d, w %d)\n",
> + __FUNCTION__, x, w);
> + w += x;
> + if (w < 0)
> + return;
> + x = 0;
> + }
> + if (y < 0) {
> + fprintf(stderr, "%s: update y was < 0 (%d, h %d)\n",
> + __FUNCTION__, y, h);
> + h += y;
> + if (h < 0)
> + return;
> + y = 0;
> + }
Looks like it has mixed spaces and tabs.
CODING_STYLE wants {} on all if's
> if (x + w > s->width) {
> fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
> __FUNCTION__, x, w);
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh@us.ibm.com
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [RFC] fix crashes with vmware driver
2012-03-02 21:20 ` Ryan Harper
@ 2012-03-05 19:33 ` Serge E. Hallyn
2012-03-05 19:33 ` [Qemu-devel] [PATCH 1/1] vmware_vga: stop crashing Serge Hallyn
1 sibling, 0 replies; 5+ messages in thread
From: Serge E. Hallyn @ 2012-03-05 19:33 UTC (permalink / raw)
To: Ryan Harper; +Cc: qemu-devel
Thanks. As there's been no substantial feedback, I'll resend with
those changes.
-serge
Quoting Ryan Harper (ryanh@us.ibm.com):
> * Serge Hallyn <serge.hallyn@canonical.com> [2012-03-02 15:13]:
> > Hi,
> >
> > I don't know where the best place to catch this would be, but
> > with vnc and vmware_vga it's possible to get set_bit called on
> > a negative index, crashing qemu. See
> >
> > https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791
> >
> > for details. This patch prevents that. It's possible this
> > should be caught earlier, but this patch works for me.
> >
> > Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com>
> > ---
> > vmware_vga.c | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > Index: qemu-kvm-1.0+noroms/hw/vmware_vga.c
> > ===================================================================
> > --- qemu-kvm-1.0+noroms.orig/hw/vmware_vga.c 2012-03-01 16:19:23.280571798 -0600
> > +++ qemu-kvm-1.0+noroms/hw/vmware_vga.c 2012-03-01 16:27:27.910975006 -0600
> > @@ -298,6 +298,22 @@
> > uint8_t *src;
> > uint8_t *dst;
> >
> > + if (x < 0) {
> > + fprintf(stderr, "%s: update x was < 0 (%d, w %d)\n",
> > + __FUNCTION__, x, w);
> > + w += x;
> > + if (w < 0)
> > + return;
> > + x = 0;
> > + }
> > + if (y < 0) {
> > + fprintf(stderr, "%s: update y was < 0 (%d, h %d)\n",
> > + __FUNCTION__, y, h);
> > + h += y;
> > + if (h < 0)
> > + return;
> > + y = 0;
> > + }
>
> Looks like it has mixed spaces and tabs.
>
> CODING_STYLE wants {} on all if's
>
>
>
> > if (x + w > s->width) {
> > fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
> > __FUNCTION__, x, w);
>
> --
> Ryan Harper
> Software Engineer; Linux Technology Center
> IBM Corp., Austin, Tx
> ryanh@us.ibm.com
>
^ permalink raw reply [flat|nested] 5+ messages in thread* [Qemu-devel] [PATCH 1/1] vmware_vga: stop crashing
2012-03-02 21:20 ` Ryan Harper
2012-03-05 19:33 ` Serge E. Hallyn
@ 2012-03-05 19:33 ` Serge Hallyn
2012-03-11 16:56 ` Gerhard Wiesinger
1 sibling, 1 reply; 5+ messages in thread
From: Serge Hallyn @ 2012-03-05 19:33 UTC (permalink / raw)
To: Ryan Harper; +Cc: qemu-devel
if x or y < 0, set them to 0 (and decrement width/height accordingly)>
I don't know where the best place to catch this would be, but
with vnc and vmware_vga it's possible to get set_bit called on
a negative index, crashing qemu. See
https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791
for details. This patch prevents that. It's possible this
should be caught earlier, but this patch works for me.
Changelog:
Mar 5: As Ryan Harper pointed out, don't mix tabs+spaces,
and put {} around all conditionals.
Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com>
---
hw/vmware_vga.c | 18 ++++++++++++++++++
1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 142d9f4..c94f9f3 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -298,6 +298,24 @@ static inline void vmsvga_update_rect(struct vmsvga_state_s *s,
uint8_t *src;
uint8_t *dst;
+ if (x < 0) {
+ fprintf(stderr, "%s: update x was < 0 (%d, w %d)\n",
+ __FUNCTION__, x, w);
+ w += x;
+ if (w < 0) {
+ return;
+ }
+ x = 0;
+ }
+ if (y < 0) {
+ fprintf(stderr, "%s: update y was < 0 (%d, h %d)\n",
+ __FUNCTION__, y, h);
+ h += y;
+ if (h < 0) {
+ return;
+ }
+ y = 0;
+ }
if (x + w > s->width) {
fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
__FUNCTION__, x, w);
--
1.7.9
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH 1/1] vmware_vga: stop crashing
2012-03-05 19:33 ` [Qemu-devel] [PATCH 1/1] vmware_vga: stop crashing Serge Hallyn
@ 2012-03-11 16:56 ` Gerhard Wiesinger
0 siblings, 0 replies; 5+ messages in thread
From: Gerhard Wiesinger @ 2012-03-11 16:56 UTC (permalink / raw)
To: Serge Hallyn; +Cc: Ryan Harper, qemu-devel
Can confirm that this patch fixes a crash which also occoured here. Since
window was out of the VNC window, crash was reproduceable and has been
removed reproduceable.
Tested-by: Gerhard Wiesinger <lists@wiesinger.com>
Please apply ASAP.
Ciao,
Gerhard
--
http://www.wiesinger.com/
On Mon, 5 Mar 2012, Serge Hallyn wrote:
>
> if x or y < 0, set them to 0 (and decrement width/height accordingly)>
>
> I don't know where the best place to catch this would be, but
> with vnc and vmware_vga it's possible to get set_bit called on
> a negative index, crashing qemu. See
>
> https://bugs.launchpad.net/ubuntu/+source/qemu-kvm/+bug/918791
>
> for details. This patch prevents that. It's possible this
> should be caught earlier, but this patch works for me.
>
> Changelog:
> Mar 5: As Ryan Harper pointed out, don't mix tabs+spaces,
> and put {} around all conditionals.
>
> Signed-off-by: Serge Hallyn <serge.hallyn@canonical.com>
> ---
> hw/vmware_vga.c | 18 ++++++++++++++++++
> 1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
> index 142d9f4..c94f9f3 100644
> --- a/hw/vmware_vga.c
> +++ b/hw/vmware_vga.c
> @@ -298,6 +298,24 @@ static inline void vmsvga_update_rect(struct vmsvga_state_s *s,
> uint8_t *src;
> uint8_t *dst;
>
> + if (x < 0) {
> + fprintf(stderr, "%s: update x was < 0 (%d, w %d)\n",
> + __FUNCTION__, x, w);
> + w += x;
> + if (w < 0) {
> + return;
> + }
> + x = 0;
> + }
> + if (y < 0) {
> + fprintf(stderr, "%s: update y was < 0 (%d, h %d)\n",
> + __FUNCTION__, y, h);
> + h += y;
> + if (h < 0) {
> + return;
> + }
> + y = 0;
> + }
> if (x + w > s->width) {
> fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
> __FUNCTION__, x, w);
> --
> 1.7.9
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-03-11 16:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-02 21:11 [Qemu-devel] [RFC] fix crashes with vmware driver Serge Hallyn
2012-03-02 21:20 ` Ryan Harper
2012-03-05 19:33 ` Serge E. Hallyn
2012-03-05 19:33 ` [Qemu-devel] [PATCH 1/1] vmware_vga: stop crashing Serge Hallyn
2012-03-11 16:56 ` Gerhard Wiesinger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).