[v2] cirrus: handle wraparound in cirrus_invalidate_region

[PATCH v2] cirrus: handle wraparound in cirrus_invalidate_region

Posted by Gerd Hoffmann 3 years, 8 months ago

Code simply asserts that there is no wraparound instead of handling
it properly.  The assert() can be triggered by the guest (must be
privilidged inside the guest though).  Fix it.

Buglink: https://bugs.launchpad.net/qemu/+bug/1880189
Cc: Li Qiang <liq3ea@163.com>
Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/display/cirrus_vga.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index 212d6f5e6145..5d1f62e50e67 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -640,10 +640,16 @@ static void cirrus_invalidate_region(CirrusVGAState * s, int off_begin,
     }
 
     for (y = 0; y < lines; y++) {
-        off_cur = off_begin;
+        off_cur = off_begin & s->cirrus_addr_mask;
         off_cur_end = ((off_cur + bytesperline - 1) & s->cirrus_addr_mask) + 1;
-        assert(off_cur_end >= off_cur);
-        memory_region_set_dirty(&s->vga.vram, off_cur, off_cur_end - off_cur);
+        if (off_cur_end >= off_cur) {
+            memory_region_set_dirty(&s->vga.vram, off_cur, off_cur_end - off_cur);
+        } else {
+            /* wraparound */
+            memory_region_set_dirty(&s->vga.vram, off_cur,
+                                    s->cirrus_addr_mask + 1 - off_cur);
+            memory_region_set_dirty(&s->vga.vram, 0, off_cur_end);
+        }
         off_begin += off_pitch;
     }
 }
-- 
2.27.0

Re: [PATCH v2] cirrus: handle wraparound in cirrus_invalidate_region

Posted by Li Qiang 3 years, 8 months ago

Gerd Hoffmann <kraxel@redhat.com> 于2020年9月1日周二 下午10:12写道：
>
> Code simply asserts that there is no wraparound instead of handling
> it properly.  The assert() can be triggered by the guest (must be
> privilidged inside the guest though).  Fix it.
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1880189
> Cc: Li Qiang <liq3ea@163.com>
> Reported-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Li Qiang <liq3ea@163.com>

> ---
>  hw/display/cirrus_vga.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> index 212d6f5e6145..5d1f62e50e67 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -640,10 +640,16 @@ static void cirrus_invalidate_region(CirrusVGAState * s, int off_begin,
>      }
>
>      for (y = 0; y < lines; y++) {
> -        off_cur = off_begin;
> +        off_cur = off_begin & s->cirrus_addr_mask;
>          off_cur_end = ((off_cur + bytesperline - 1) & s->cirrus_addr_mask) + 1;
> -        assert(off_cur_end >= off_cur);
> -        memory_region_set_dirty(&s->vga.vram, off_cur, off_cur_end - off_cur);
> +        if (off_cur_end >= off_cur) {
> +            memory_region_set_dirty(&s->vga.vram, off_cur, off_cur_end - off_cur);
> +        } else {
> +            /* wraparound */
> +            memory_region_set_dirty(&s->vga.vram, off_cur,
> +                                    s->cirrus_addr_mask + 1 - off_cur);
> +            memory_region_set_dirty(&s->vga.vram, 0, off_cur_end);
> +        }
>          off_begin += off_pitch;
>      }
>  }
> --
> 2.27.0
>
>