diff mbox series

ati-vga: Fix check for blt outside vram

Message ID 20190409110732.5C5FF7465DB@zero.eik.bme.hu
State New
Headers show
Series ati-vga: Fix check for blt outside vram | expand

Commit Message

BALATON Zoltan April 9, 2019, 10:56 a.m. UTC 
Fix the check preventing calling pixman functions that would access
memory outside allocated vram. The r128 X driver sometimes seem to try
blits that span outside vram, this check prevents crashing QEMU in
that case. (The r128 X driver may have problems even on real hardware
so I'm not sure if it's a client bug or emulation problem but at least
QEMU should survive.)

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Tested-by: Andrew Randrianasulu <randrianasulu@gmail.com>
---
 hw/display/ati_2d.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Philippe Mathieu-Daudé April 9, 2019, 1:03 p.m. UTC | #1 
This patch looks 4.0 worthwhile.

On 4/9/19 12:56 PM, BALATON Zoltan wrote:
> Fix the check preventing calling pixman functions that would access
> memory outside allocated vram. The r128 X driver sometimes seem to try
> blits that span outside vram, this check prevents crashing QEMU in
> that case. (The r128 X driver may have problems even on real hardware
> so I'm not sure if it's a client bug or emulation problem but at least
> QEMU should survive.)
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> Tested-by: Andrew Randrianasulu <randrianasulu@gmail.com>
> ---
>  hw/display/ati_2d.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
> index bc98ba6eeb..fe3ae14864 100644
> --- a/hw/display/ati_2d.c
> +++ b/hw/display/ati_2d.c
> @@ -79,10 +79,10 @@ void ati_2d_blt(ATIVGAState *s)
>                  s->regs.dst_width, s->regs.dst_height);
>          end = s->vga.vram_ptr + s->vga.vram_size;
>          if (src_bits >= end || dst_bits >= end ||
> -            src_bits + (s->regs.src_y + s->regs.dst_height) * src_stride +
> -            s->regs.src_x >= end ||
> -            dst_bits + (s->regs.dst_y + s->regs.dst_height) * dst_stride +
> -            s->regs.dst_x >= end) {
> +            src_bits + s->regs.src_x + (s->regs.src_y + s->regs.dst_height) *
> +            src_stride * sizeof(uint32_t) >= end ||
> +            dst_bits + s->regs.dst_x + (s->regs.dst_y + s->regs.dst_height) *
> +            dst_stride * sizeof(uint32_t) >= end) {
>              qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
>              return;
>          }
> @@ -140,8 +140,8 @@ void ati_2d_blt(ATIVGAState *s)
>                  filler);
>          end = s->vga.vram_ptr + s->vga.vram_size;
>          if (dst_bits >= end ||
> -            dst_bits + (s->regs.dst_y + s->regs.dst_height) * dst_stride +
> -            s->regs.dst_x >= end) {
> +            dst_bits + s->regs.dst_x + (s->regs.dst_y + s->regs.dst_height) *
> +            dst_stride * sizeof(uint32_t) >= end) {
>              qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
>              return;
>          }
>
BALATON Zoltan April 14, 2019, 8:30 p.m. UTC | #2 
On Tue, 9 Apr 2019, Philippe Mathieu-Daudé wrote:
> This patch looks 4.0 worthwhile.

Now that it seems we'll have another rc, will this get in? Gerd, I think 
you have to send a pull request with it for that.

Regards,
BALATON Zoltan

> On 4/9/19 12:56 PM, BALATON Zoltan wrote:
>> Fix the check preventing calling pixman functions that would access
>> memory outside allocated vram. The r128 X driver sometimes seem to try
>> blits that span outside vram, this check prevents crashing QEMU in
>> that case. (The r128 X driver may have problems even on real hardware
>> so I'm not sure if it's a client bug or emulation problem but at least
>> QEMU should survive.)
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> Tested-by: Andrew Randrianasulu <randrianasulu@gmail.com>
>> ---
>>  hw/display/ati_2d.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
>> index bc98ba6eeb..fe3ae14864 100644
>> --- a/hw/display/ati_2d.c
>> +++ b/hw/display/ati_2d.c
>> @@ -79,10 +79,10 @@ void ati_2d_blt(ATIVGAState *s)
>>                  s->regs.dst_width, s->regs.dst_height);
>>          end = s->vga.vram_ptr + s->vga.vram_size;
>>          if (src_bits >= end || dst_bits >= end ||
>> -            src_bits + (s->regs.src_y + s->regs.dst_height) * src_stride +
>> -            s->regs.src_x >= end ||
>> -            dst_bits + (s->regs.dst_y + s->regs.dst_height) * dst_stride +
>> -            s->regs.dst_x >= end) {
>> +            src_bits + s->regs.src_x + (s->regs.src_y + s->regs.dst_height) *
>> +            src_stride * sizeof(uint32_t) >= end ||
>> +            dst_bits + s->regs.dst_x + (s->regs.dst_y + s->regs.dst_height) *
>> +            dst_stride * sizeof(uint32_t) >= end) {
>>              qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
>>              return;
>>          }
>> @@ -140,8 +140,8 @@ void ati_2d_blt(ATIVGAState *s)
>>                  filler);
>>          end = s->vga.vram_ptr + s->vga.vram_size;
>>          if (dst_bits >= end ||
>> -            dst_bits + (s->regs.dst_y + s->regs.dst_height) * dst_stride +
>> -            s->regs.dst_x >= end) {
>> +            dst_bits + s->regs.dst_x + (s->regs.dst_y + s->regs.dst_height) *
>> +            dst_stride * sizeof(uint32_t) >= end) {
>>              qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
>>              return;
>>          }
>>
>
>
BALATON Zoltan April 25, 2019, 10:51 p.m. UTC | #3 
On Sun, 14 Apr 2019, BALATON Zoltan wrote:
> On Tue, 9 Apr 2019, Philippe Mathieu-Daudé wrote:
>> This patch looks 4.0 worthwhile.
>
> Now that it seems we'll have another rc, will this get in? Gerd, I think you 
> have to send a pull request with it for that.

Ping? This has missed two rc-s. This prevents a crash so you might want to 
queue it for stable as well now.

Regards,
BALATON Zoltan

>
>> On 4/9/19 12:56 PM, BALATON Zoltan wrote:
>>> Fix the check preventing calling pixman functions that would access
>>> memory outside allocated vram. The r128 X driver sometimes seem to try
>>> blits that span outside vram, this check prevents crashing QEMU in
>>> that case. (The r128 X driver may have problems even on real hardware
>>> so I'm not sure if it's a client bug or emulation problem but at least
>>> QEMU should survive.)
>>> 
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> Tested-by: Andrew Randrianasulu <randrianasulu@gmail.com>
>>> ---
>>>  hw/display/ati_2d.c | 12 ++++++------
>>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
>>> index bc98ba6eeb..fe3ae14864 100644
>>> --- a/hw/display/ati_2d.c
>>> +++ b/hw/display/ati_2d.c
>>> @@ -79,10 +79,10 @@ void ati_2d_blt(ATIVGAState *s)
>>>                  s->regs.dst_width, s->regs.dst_height);
>>>          end = s->vga.vram_ptr + s->vga.vram_size;
>>>          if (src_bits >= end || dst_bits >= end ||
>>> -            src_bits + (s->regs.src_y + s->regs.dst_height) * src_stride 
>>> +
>>> -            s->regs.src_x >= end ||
>>> -            dst_bits + (s->regs.dst_y + s->regs.dst_height) * dst_stride 
>>> +
>>> -            s->regs.dst_x >= end) {
>>> +            src_bits + s->regs.src_x + (s->regs.src_y + 
>>> s->regs.dst_height) *
>>> +            src_stride * sizeof(uint32_t) >= end ||
>>> +            dst_bits + s->regs.dst_x + (s->regs.dst_y + 
>>> s->regs.dst_height) *
>>> +            dst_stride * sizeof(uint32_t) >= end) {
>>>              qemu_log_mask(LOG_UNIMP, "blt outside vram not 
>>> implemented\n");
>>>              return;
>>>          }
>>> @@ -140,8 +140,8 @@ void ati_2d_blt(ATIVGAState *s)
>>>                  filler);
>>>          end = s->vga.vram_ptr + s->vga.vram_size;
>>>          if (dst_bits >= end ||
>>> -            dst_bits + (s->regs.dst_y + s->regs.dst_height) * dst_stride 
>>> +
>>> -            s->regs.dst_x >= end) {
>>> +            dst_bits + s->regs.dst_x + (s->regs.dst_y + 
>>> s->regs.dst_height) *
>>> +            dst_stride * sizeof(uint32_t) >= end) {
>>>              qemu_log_mask(LOG_UNIMP, "blt outside vram not 
>>> implemented\n");
>>>              return;
>>>          }
>>> 
>> 
>> 
>
>
Gerd Hoffmann May 7, 2019, 7:55 a.m. UTC | #4 
On Tue, Apr 09, 2019 at 12:56:18PM +0200, BALATON Zoltan wrote:
> Fix the check preventing calling pixman functions that would access
> memory outside allocated vram. The r128 X driver sometimes seem to try
> blits that span outside vram, this check prevents crashing QEMU in
> that case. (The r128 X driver may have problems even on real hardware
> so I'm not sure if it's a client bug or emulation problem but at least
> QEMU should survive.)

Added to vga queue.

thanks,
  Gerd
diff mbox series

Patch

diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index bc98ba6eeb..fe3ae14864 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -79,10 +79,10 @@  void ati_2d_blt(ATIVGAState *s)
                 s->regs.dst_width, s->regs.dst_height);
         end = s->vga.vram_ptr + s->vga.vram_size;
         if (src_bits >= end || dst_bits >= end ||
-            src_bits + (s->regs.src_y + s->regs.dst_height) * src_stride +
-            s->regs.src_x >= end ||
-            dst_bits + (s->regs.dst_y + s->regs.dst_height) * dst_stride +
-            s->regs.dst_x >= end) {
+            src_bits + s->regs.src_x + (s->regs.src_y + s->regs.dst_height) *
+            src_stride * sizeof(uint32_t) >= end ||
+            dst_bits + s->regs.dst_x + (s->regs.dst_y + s->regs.dst_height) *
+            dst_stride * sizeof(uint32_t) >= end) {
             qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
             return;
         }
@@ -140,8 +140,8 @@  void ati_2d_blt(ATIVGAState *s)
                 filler);
         end = s->vga.vram_ptr + s->vga.vram_size;
         if (dst_bits >= end ||
-            dst_bits + (s->regs.dst_y + s->regs.dst_height) * dst_stride +
-            s->regs.dst_x >= end) {
+            dst_bits + s->regs.dst_x + (s->regs.dst_y + s->regs.dst_height) *
+            dst_stride * sizeof(uint32_t) >= end) {
             qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n");
             return;
         }