Re: [PATCH] cirrus: handle wraparound in cirrus_invalidate_region

[Philippe Mathieu-Daudé]

On 8/21/20 10:26 AM, Gerd Hoffmann wrote:
> 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.

Thanks for fixing this!

> Buglink: https://bugs.launchpad.net/qemu/+bug/1880189
> Cc: Li Qiang <liq3ea@163.com>
> Reported-by: Alexander Bulekov <alxndr@bu.edu>

Well, I reported this one 3 months ago to secalert, not Alex.

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

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/display/cirrus_vga.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> index 212d6f5e6145..b91b64347473 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -640,10 +640,15 @@ 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 - off_cur);
> +            memory_region_set_dirty(&s->vga.vram, 0, off_cur_end);
> +        }
>          off_begin += off_pitch;
>      }
>  }
>