1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v3 0/2] ati: fix ati_cursor_define bug.
  4. View patch

[Qemu-devel] [PATCH v3 2/2] ati: use vga_read_byte in ati_cursor_define

Gerd Hoffmann posted 2 patches 6 years ago
Maintainers: Gerd Hoffmann <kraxel@redhat.com>
[Qemu-devel] [PATCH v3 2/2] ati: use vga_read_byte in ati_cursor_define
Posted by Gerd Hoffmann 6 years ago 
This makes sure reads are confined to vga video memory.

v3: use uint32_t, fix cut+paste bug.
v2: fix ati_cursor_draw_line too.

Reported-by: xu hang <flier_m@outlook.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/ati.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/hw/display/ati.c b/hw/display/ati.c
index 8f940eee221a..db3b2543163f 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -19,6 +19,7 @@
 #include "qemu/osdep.h"
 #include "ati_int.h"
 #include "ati_regs.h"
+#include "vga-access.h"
 #include "hw/qdev-properties.h"
 #include "vga_regs.h"
 #include "qemu/log.h"
@@ -135,19 +136,19 @@ static void ati_vga_switch_mode(ATIVGAState *s)
 static void ati_cursor_define(ATIVGAState *s)
 {
     uint8_t data[1024];
-    uint8_t *src;
+    uint32_t srcoff;
     int i, j, idx = 0;
 
     if ((s->regs.cur_offset & BIT(31)) || s->cursor_guest_mode) {
         return; /* Do not update cursor if locked or rendered by guest */
     }
     /* FIXME handle cur_hv_offs correctly */
-    src = s->vga.vram_ptr + s->regs.cur_offset -
-          (s->regs.cur_hv_offs >> 16) - (s->regs.cur_hv_offs & 0xffff) * 16;
+    srcoff = s->regs.cur_offset -
+        (s->regs.cur_hv_offs >> 16) - (s->regs.cur_hv_offs & 0xffff) * 16;
     for (i = 0; i < 64; i++) {
         for (j = 0; j < 8; j++, idx++) {
-            data[idx] = src[i * 16 + j];
-            data[512 + idx] = src[i * 16 + j + 8];
+            data[idx] = vga_read_byte(&s->vga, srcoff + i * 16 + j);
+            data[512 + idx] = vga_read_byte(&s->vga, srcoff + i * 16 + j + 8);
         }
     }
     if (!s->cursor) {
@@ -189,7 +190,7 @@ static void ati_cursor_invalidate(VGACommonState *vga)
 static void ati_cursor_draw_line(VGACommonState *vga, uint8_t *d, int scr_y)
 {
     ATIVGAState *s = container_of(vga, ATIVGAState, vga);
-    uint8_t *src;
+    uint32_t srcoff;
     uint32_t *dp = (uint32_t *)d;
     int i, j, h;
 
@@ -199,13 +200,13 @@ static void ati_cursor_draw_line(VGACommonState *vga, uint8_t *d, int scr_y)
         return;
     }
     /* FIXME handle cur_hv_offs correctly */
-    src = s->vga.vram_ptr + s->cursor_offset + (scr_y - vga->hw_cursor_y) * 16;
+    srcoff = s->cursor_offset + (scr_y - vga->hw_cursor_y) * 16;
     dp = &dp[vga->hw_cursor_x];
     h = ((s->regs.crtc_h_total_disp >> 16) + 1) * 8;
     for (i = 0; i < 8; i++) {
         uint32_t color;
-        uint8_t abits = src[i];
-        uint8_t xbits = src[i + 8];
+        uint8_t abits = vga_read_byte(vga, srcoff + i);
+        uint8_t xbits = vga_read_byte(vga, srcoff + i + 8);
         for (j = 0; j < 8; j++, abits <<= 1, xbits <<= 1) {
             if (abits & BIT(7)) {
                 if (xbits & BIT(7)) {
-- 
2.18.1
Re: [Qemu-devel] [PATCH v3 2/2] ati: use vga_read_byte in ati_cursor_define
Posted by BALATON Zoltan 6 years ago 
On Tue, 17 Sep 2019, Gerd Hoffmann wrote:
> This makes sure reads are confined to vga video memory.
>
> v3: use uint32_t, fix cut+paste bug.
> v2: fix ati_cursor_draw_line too.
>
> Reported-by: xu hang <flier_m@outlook.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

Thank you,
BALATON Zoltan

> ---
> hw/display/ati.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/hw/display/ati.c b/hw/display/ati.c
> index 8f940eee221a..db3b2543163f 100644
> --- a/hw/display/ati.c
> +++ b/hw/display/ati.c
> @@ -19,6 +19,7 @@
> #include "qemu/osdep.h"
> #include "ati_int.h"
> #include "ati_regs.h"
> +#include "vga-access.h"
> #include "hw/qdev-properties.h"
> #include "vga_regs.h"
> #include "qemu/log.h"
> @@ -135,19 +136,19 @@ static void ati_vga_switch_mode(ATIVGAState *s)
> static void ati_cursor_define(ATIVGAState *s)
> {
>     uint8_t data[1024];
> -    uint8_t *src;
> +    uint32_t srcoff;
>     int i, j, idx = 0;
>
>     if ((s->regs.cur_offset & BIT(31)) || s->cursor_guest_mode) {
>         return; /* Do not update cursor if locked or rendered by guest */
>     }
>     /* FIXME handle cur_hv_offs correctly */
> -    src = s->vga.vram_ptr + s->regs.cur_offset -
> -          (s->regs.cur_hv_offs >> 16) - (s->regs.cur_hv_offs & 0xffff) * 16;
> +    srcoff = s->regs.cur_offset -
> +        (s->regs.cur_hv_offs >> 16) - (s->regs.cur_hv_offs & 0xffff) * 16;
>     for (i = 0; i < 64; i++) {
>         for (j = 0; j < 8; j++, idx++) {
> -            data[idx] = src[i * 16 + j];
> -            data[512 + idx] = src[i * 16 + j + 8];
> +            data[idx] = vga_read_byte(&s->vga, srcoff + i * 16 + j);
> +            data[512 + idx] = vga_read_byte(&s->vga, srcoff + i * 16 + j + 8);
>         }
>     }
>     if (!s->cursor) {
> @@ -189,7 +190,7 @@ static void ati_cursor_invalidate(VGACommonState *vga)
> static void ati_cursor_draw_line(VGACommonState *vga, uint8_t *d, int scr_y)
> {
>     ATIVGAState *s = container_of(vga, ATIVGAState, vga);
> -    uint8_t *src;
> +    uint32_t srcoff;
>     uint32_t *dp = (uint32_t *)d;
>     int i, j, h;
>
> @@ -199,13 +200,13 @@ static void ati_cursor_draw_line(VGACommonState *vga, uint8_t *d, int scr_y)
>         return;
>     }
>     /* FIXME handle cur_hv_offs correctly */
> -    src = s->vga.vram_ptr + s->cursor_offset + (scr_y - vga->hw_cursor_y) * 16;
> +    srcoff = s->cursor_offset + (scr_y - vga->hw_cursor_y) * 16;
>     dp = &dp[vga->hw_cursor_x];
>     h = ((s->regs.crtc_h_total_disp >> 16) + 1) * 8;
>     for (i = 0; i < 8; i++) {
>         uint32_t color;
> -        uint8_t abits = src[i];
> -        uint8_t xbits = src[i + 8];
> +        uint8_t abits = vga_read_byte(vga, srcoff + i);
> +        uint8_t xbits = vga_read_byte(vga, srcoff + i + 8);
>         for (j = 0; j < 8; j++, abits <<= 1, xbits <<= 1) {
>             if (abits & BIT(7)) {
>                 if (xbits & BIT(7)) {
>
Re: [Qemu-devel] [PATCH v3 2/2] ati: use vga_read_byte in ati_cursor_define
Posted by Philippe Mathieu-Daudé 6 years ago 
On 9/17/19 1:14 PM, Gerd Hoffmann wrote:
> This makes sure reads are confined to vga video memory.
> 
> v3: use uint32_t, fix cut+paste bug.
> v2: fix ati_cursor_draw_line too.
> 
> Reported-by: xu hang <flier_m@outlook.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/display/ati.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/display/ati.c b/hw/display/ati.c
> index 8f940eee221a..db3b2543163f 100644
> --- a/hw/display/ati.c
> +++ b/hw/display/ati.c
> @@ -19,6 +19,7 @@
>  #include "qemu/osdep.h"
>  #include "ati_int.h"
>  #include "ati_regs.h"
> +#include "vga-access.h"
>  #include "hw/qdev-properties.h"
>  #include "vga_regs.h"
>  #include "qemu/log.h"
> @@ -135,19 +136,19 @@ static void ati_vga_switch_mode(ATIVGAState *s)
>  static void ati_cursor_define(ATIVGAState *s)
>  {
>      uint8_t data[1024];
> -    uint8_t *src;
> +    uint32_t srcoff;
>      int i, j, idx = 0;
>  
>      if ((s->regs.cur_offset & BIT(31)) || s->cursor_guest_mode) {
>          return; /* Do not update cursor if locked or rendered by guest */
>      }
>      /* FIXME handle cur_hv_offs correctly */
> -    src = s->vga.vram_ptr + s->regs.cur_offset -
> -          (s->regs.cur_hv_offs >> 16) - (s->regs.cur_hv_offs & 0xffff) * 16;
> +    srcoff = s->regs.cur_offset -
> +        (s->regs.cur_hv_offs >> 16) - (s->regs.cur_hv_offs & 0xffff) * 16;
>      for (i = 0; i < 64; i++) {
>          for (j = 0; j < 8; j++, idx++) {
> -            data[idx] = src[i * 16 + j];
> -            data[512 + idx] = src[i * 16 + j + 8];
> +            data[idx] = vga_read_byte(&s->vga, srcoff + i * 16 + j);
> +            data[512 + idx] = vga_read_byte(&s->vga, srcoff + i * 16 + j + 8);
>          }
>      }
>      if (!s->cursor) {
> @@ -189,7 +190,7 @@ static void ati_cursor_invalidate(VGACommonState *vga)
>  static void ati_cursor_draw_line(VGACommonState *vga, uint8_t *d, int scr_y)
>  {
>      ATIVGAState *s = container_of(vga, ATIVGAState, vga);
> -    uint8_t *src;
> +    uint32_t srcoff;
>      uint32_t *dp = (uint32_t *)d;
>      int i, j, h;
>  
> @@ -199,13 +200,13 @@ static void ati_cursor_draw_line(VGACommonState *vga, uint8_t *d, int scr_y)
>          return;
>      }
>      /* FIXME handle cur_hv_offs correctly */
> -    src = s->vga.vram_ptr + s->cursor_offset + (scr_y - vga->hw_cursor_y) * 16;
> +    srcoff = s->cursor_offset + (scr_y - vga->hw_cursor_y) * 16;
>      dp = &dp[vga->hw_cursor_x];
>      h = ((s->regs.crtc_h_total_disp >> 16) + 1) * 8;
>      for (i = 0; i < 8; i++) {
>          uint32_t color;
> -        uint8_t abits = src[i];
> -        uint8_t xbits = src[i + 8];
> +        uint8_t abits = vga_read_byte(vga, srcoff + i);
> +        uint8_t xbits = vga_read_byte(vga, srcoff + i + 8);
>          for (j = 0; j < 8; j++, abits <<= 1, xbits <<= 1) {
>              if (abits & BIT(7)) {
>                  if (xbits & BIT(7)) {
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.