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

[Michael Tokarev]

On 17.09.2012 00:36, Marek Vasut wrote:
>> Dear Michael Tokarev,
> 
> Bump? Did this lead anywhere? Do you need updated patch?


This didn't lead to anywhere.  I just pointed out that we
have similar issue and somewhat similar change, and it
weren't sufficient.  I don't know this code, and have no
idea who does, either.

Thanks,

/mjt

>>> 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 <address@hidden>
>>>> ---
>>>>
>>>>  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/2
>>> 1
>>
>> 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
> 
> Best regards,
> Marek Vasut