| Message ID | 20170913074140.5160-2-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,1/6] vga: fix display update region calculation (split screen) | expand |
On 13 September 2017 at 08:41, Gerd Hoffmann <kraxel@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 <ppandit@redhat.com> > Reported-by: David Buchanan <d@vidbuchanan.co.uk> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > Message-id: 20170828123307.15392-1-kraxel@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
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); }
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 <ppandit@redhat.com> Reported-by: David Buchanan <d@vidbuchanan.co.uk> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 20170828123307.15392-1-kraxel@redhat.com --- hw/display/vga.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)