Re: [Qemu-devel] [PULL 1/6] vga: fix display update region calculation (split screen)

[Peter Maydell]

On 13 September 2017 at 08:41, Gerd Hoffmann <address@hidden> wrote:
> vga display update mis-calculated the region for the dirty bitmap
> snapshot in case split screen mode is used.  This can trigger an
> assert in cpu_physical_memory_snapshot_get_dirty().
>
> Impact:  DoS for privileged guest users.
>
> Fixes: CVE-2017-13673
> Fixes: fec5e8c92becad223df9d972770522f64aafdb72
> Cc: P J P <address@hidden>
> Reported-by: David Buchanan <address@hidden>
> Signed-off-by: Gerd Hoffmann <address@hidden>
> Message-id: address@hidden
> ---
>  hw/display/vga.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/hw/display/vga.c b/hw/display/vga.c
> index 3433102ef3..ad7a46563c 100644
> --- a/hw/display/vga.c
> +++ b/hw/display/vga.c
> @@ -1628,9 +1628,15 @@ static void vga_draw_graphic(VGACommonState *s, int 
> full_update)
>      y1 = 0;
>
>      if (!full_update) {
> +        ram_addr_t region_start = addr1;
> +        ram_addr_t region_end = addr1 + line_offset * height;
>          vga_sync_dirty_bitmap(s);
> -        snap = memory_region_snapshot_and_clear_dirty(&s->vram, addr1,
> -                                                      line_offset * height,
> +        if (s->line_compare < height) {
> +            /* split screen mode */
> +            region_start = 0;
> +        }
> +        snap = memory_region_snapshot_and_clear_dirty(&s->vram, region_start,
> +                                                      region_end - 
> region_start,
>                                                        DIRTY_MEMORY_VGA);
>      }

Hi; Coverity complains about this change, that
        ram_addr_t region_end = addr1 + line_offset * height;

is a potential overflow-before-widen because we calculate
line_offset * height as a 32x32 bit multiply and then
assign it to a 64 bit variable, rather than doing a
64 bit multiply. We could shut it up by casting line_offset
to ram_addr_t...

I don't know whether it's actually possible in this case
for the input values to be such that the multiply result
is bigger than 32 bits.

(CID 1381409.)

thanks
-- PMM