Re: [Qemu-devel] [PATCH] vmware_vga: Redraw only visible area

[Marek Vasut <marex@denx.de>]

Dear Michael Tokarev,

> On 17.08.2012 06:55, Marek Vasut wrote:
> > Disallow negative value boundaries of the redraw rectangle.
> > This fixes a segfault when using -vga vmware.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > ---
> > 
> >  hw/vmware_vga.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > NOTE: I tested this by emulating some recent version of ubuntu. The
> > rect->x
> > 
> >       value was set to -65 for some reason at one point, which caused the
> >       kvm to crash. Trimming the rectangle fixed the issue.
> > 
> > diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
> > index f5e4f44..62e5887 100644
> > --- a/hw/vmware_vga.c
> > +++ b/hw/vmware_vga.c
> > @@ -337,8 +337,8 @@ static inline void vmsvga_update_rect_delayed(struct
> > vmsvga_state_s *s,
> > 
> >  {
> >  
> >      struct vmsvga_rect_s *rect = &s->redraw_fifo[s->redraw_fifo_last
> >      ++]; s->redraw_fifo_last &= REDRAW_FIFO_LEN - 1;
> > 
> > -    rect->x = x;
> > -    rect->y = y;
> > +    rect->x = (x < 0) ? 0 : x;
> > +    rect->y = (y < 0) ? 0 : y;
> > 
> >      rect->w = w;
> >      rect->h = h;
> >  
> >  }
> 
> 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

Looks to be the same ... though my patch tries to squash the issue as early as 
possible.

You're right that x and y might overflow to the other side too. Also, you're 
right about w and h.

Shall I send updated patch?

> "So indeed, some (upstream) verification is needed here -- where these
> negative values are coming from, whenever it is EVER okay to have them,
> what to do with these, and where to check (I guess the check should be
> done somewhere in the upper layer)."
> 
> Especially the last part about the layer.

Where's the upper layer though, isn't that what's pouring out of the virtual 
machine itself?

> Thanks,

Thank you for guidance !

> /mjt

Best regards,
Marek Vasut