On 13 September 2017 at 08:41, Gerd Hoffmann <kra...@redhat.com> 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 <ppan...@redhat.com>
> Reported-by: David Buchanan <d...@vidbuchanan.co.uk>
> Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
> Message-id: 20170828123307.15392-1-kra...@redhat.com
> ---
> 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
