[Qemu-devel] qemu vga crash

[Qemu-devel] qemu vga crash

[Vladimir Sementsov-Ogievskiy]

Hi Gerd!

Writing to you, as you were the last one who committed to vga_draw_graphic, hope you can help.

We faced the following crash in 2.12-based Qemu, but code seems not really changed:

#0  __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  __GI_abort () at abort.c:90
#2  __assert_fail_base (
     fmt=0x7f01126b9520 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
     assertion=assertion@entry=0x5612f995fff6 "start + length <= snap->end",
     file=file@entry=0x5612f99601d0 "/builddir/build/BUILD/qemu-2.10.0/exec.c", line=line@entry=1198,
     function=function@entry=0x5612f9960b60 <__PRETTY_FUNCTION__.33659> "cpu_physical_memory_snapshot_get_dirty") at assert.c:92
#3  __GI___assert_fail (
     assertion=assertion@entry=0x5612f995fff6 "start + length <= snap->end",
     file=file@entry=0x5612f99601d0 "/builddir/build/BUILD/qemu-2.10.0/exec.c", line=line@entry=1198,
     function=function@entry=0x5612f9960b60 <__PRETTY_FUNCTION__.33659> "cpu_physical_memory_snapshot_get_dirty") at assert.c:101
#4  cpu_physical_memory_snapshot_get_dirty (snap=snap@entry=0x5613087bdcb0,
     start=<optimized out>, length=<optimized out>) at /usr/src/debug/qemu-2.10.0/exec.c:1198
#5  memory_region_snapshot_get_dirty (mr=mr@entry=0x5613092e08f0,
     snap=snap@entry=0x5613087bdcb0, addr=<optimized out>, size=<optimized out>)
     at /usr/src/debug/qemu-2.10.0/memory.c:1949
#6  vga_draw_graphic (full_update=0, s=0x5613092e08e0)
     at /usr/src/debug/qemu-2.10.0/hw/display/vga.c:1678
#7  vga_update_display (opaque=0x5613092e08e0) at /usr/src/debug/qemu-2.10.0/hw/display/vga.c:1774
#8  vnc_refresh (dcl=0x561309116060) at ui/vnc.c:3013
#9  dpy_refresh (s=0x5613088f8e10) at ui/console.c:1613
#10 gui_update (opaque=0x5613088f8e10) at ui/console.c:201
#11 timerlist_run_timers (timer_list=0x5612fba05340) at util/qemu-timer.c:536
#12 qemu_clock_run_timers (type=<optimized out>) at util/qemu-timer.c:547
#13 qemu_clock_run_all_timers () at util/qemu-timer.c:662
#14 main_loop_wait (nonblocking=nonblocking@entry=0) at util/main-loop.c:521
#15 main_loop () at vl.c:1937
#16 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:4829


It's an assertion "assert(start + length <= snap->end);" in cpu_physical_memory_snapshot_get_dirty,
and seems that start = snap->end, when length is 511.

Digging in, I found, that in vga_draw_graphic we most probably have s->get_bpp() returning zero, which leads
to region_end = region_start, i.e. region of zero length, which then leads to snap->end == snap->start == s->vram.ram_block->offset.

On the other hand

page0 = addr & s->vbe_size_mask = 0
page1 = (addr + bwidth - 1) & s->vbe_size_mask = 511

which finally leads to assert fail in cpu_physical_memory_snapshot_get_dirty()

(addr is 0, bwidth is 512, width is 1024 and height is 768)..


I don't understand, what get_bpp = 0 means? For me it looks strange (it is bits per pixel, isn't it?)..
Hope you can shed some light on this.


=============== some additional debugging information if it helps ========

(gdb) fr 4
#4  0x00005612f95d7a11 in cpu_physical_memory_snapshot_get_dirty (snap=snap@entry=0x5613087bdcb0,
     start=<optimized out>, length=<optimized out>) at /usr/src/debug/qemu-2.10.0/exec.c:1198
1198        assert(start + length <= snap->end);
(gdb) list
1193                                                ram_addr_t length)
1194    {
1195        unsigned long page, end;
1196
1197        assert(start >= snap->start);
1198        assert(start + length <= snap->end);
1199
1200        end = TARGET_PAGE_ALIGN(start + length - snap->start) >> TARGET_PAGE_BITS;
1201        page = (start - snap->start) >> TARGET_PAGE_BITS;
1202
(gdb) info locals
page = <optimized out>
end = <optimized out>
__PRETTY_FUNCTION__ = "cpu_physical_memory_snapshot_get_dirty"
(gdb) p snap
$61 = (DirtyBitmapSnapshot *) 0x5613087bdcb0
(gdb) p *snap
$62 = {start = 77310459904, end = 77310459904, dirty = 0x5613087bdcc0}


(gdb) fr 5
#5  0x00005612f9621d2e in memory_region_snapshot_get_dirty (mr=mr@entry=0x5613092e08f0,
     snap=snap@entry=0x5613087bdcb0, addr=<optimized out>, size=<optimized out>)
     at /usr/src/debug/qemu-2.10.0/memory.c:1949
1949        return cpu_physical_memory_snapshot_get_dirty(snap,
(gdb) list
1944
1945    bool memory_region_snapshot_get_dirty(MemoryRegion *mr, DirtyBitmapSnapshot *snap,
1946                                          hwaddr addr, hwaddr size)
1947    {
1948        assert(mr->ram_block);
1949        return cpu_physical_memory_snapshot_get_dirty(snap,
1950                    memory_region_get_ram_addr(mr) + addr, size);
1951    }
1952
1953    void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
(gdb) info locals
__PRETTY_FUNCTION__ = "memory_region_snapshot_get_dirty"
(gdb) p *mr->ram_block
$63 = {rcu = {next = 0x0, func = 0x0}, mr = 0x5613092e08f0,
   host = 0x7eeef9000000 <Address 0x7eeef9000000 out of bounds>, offset = 77310459904,
   used_length = 33554432, max_length = 33554432, resized = 0x0, flags = 0,
   idstr = "vga.vram", '\000' <repeats 247 times>, next = {le_next = 0x5613075fca80,
     le_prev = 0x5612fba2c748}, ramblock_notifiers = {lh_first = 0x0}, fd = -1, page_size = 4096,
   bmap = 0x0, unsentmap = 0x0}


(gdb) fr 6
#6  0x00005612f9644af5 in vga_draw_graphic (full_update=0, s=0x5613092e08e0)
     at /usr/src/debug/qemu-2.10.0/hw/display/vga.c:1678
1678                update = memory_region_snapshot_get_dirty(&s->vram, snap,
(gdb) list
1673                update = memory_region_snapshot_get_dirty(&s->vram, snap,
1674                                                          page0, 0);
1675                update |= memory_region_snapshot_get_dirty(&s->vram, snap,
1676                                                           page1, 0);
1677            } else {
1678                update = memory_region_snapshot_get_dirty(&s->vram, snap,
1679                                                          page0, page1 - page0);
1680            }
1681            /* explicit invalidation for the hardware cursor (cirrus only) */
1682            update |= vga_scanline_invalidated(s, y);
(gdb) info locals
double_scan = <optimized out>
width = 1024
multi_scan = 0
y = 0
bits = <optimized out>
snap = 0x5613087bdcb0
byteswap = <optimized out>
region_end = <optimized out>
addr1 = 0
addr = 0
share_surface = <optimized out>
format = <optimized out>
y1 = 0
y_start = -1
multi_run = 0
surface = 0x561308ac5c80
update = 0
depth = <optimized out>
height = 768
bwidth = 512
region_start = <optimized out>
vga_draw_line = 0x5612f96418d0 <vga_draw_line8d2>
mask = <optimized out>
page0 = <optimized out>
d = 0x56130a68c000 ""
v = <optimized out>
force_shadow = false
shift_control = <optimized out>
page1 = <optimized out>
disp_width = 1024

(gdb) p full_update
$64 = 0
(gdb) p s->line_offset
$65 = 0
(gdb) p s->get_bpp
$66 = (int (*)(struct VGACommonState *)) 0x5612f9641f60 <vga_get_bpp>
(gdb) p s->vbe_regs
$67 = {45248, 1024, 768, 32, 0, 0, 1024, 8192, 0, 0}  // which means that vbe_enabled should return 0 and then vga_get_bpp should return 0
(gdb) p (s->gr[5] >> 5) & 3
$68 = 2              // it should be shift_control


-- 
Best regards,
Vladimir

Re: [Qemu-devel] qemu vga crash

[Vladimir Sementsov-Ogievskiy]

Could anybody help?

15.05.2019 15:28, Vladimir Sementsov-Ogievskiy wrote:
> Hi Gerd!
> 
> Writing to you, as you were the last one who committed to vga_draw_graphic, hope you can help.
> 
> We faced the following crash in 2.12-based Qemu, but code seems not really changed:
> 
> #0  __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
> #1  __GI_abort () at abort.c:90
> #2  __assert_fail_base (
>      fmt=0x7f01126b9520 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
>      assertion=assertion@entry=0x5612f995fff6 "start + length <= snap->end",
>      file=file@entry=0x5612f99601d0 "/builddir/build/BUILD/qemu-2.10.0/exec.c", line=line@entry=1198,
>      function=function@entry=0x5612f9960b60 <__PRETTY_FUNCTION__.33659> "cpu_physical_memory_snapshot_get_dirty") at assert.c:92
> #3  __GI___assert_fail (
>      assertion=assertion@entry=0x5612f995fff6 "start + length <= snap->end",
>      file=file@entry=0x5612f99601d0 "/builddir/build/BUILD/qemu-2.10.0/exec.c", line=line@entry=1198,
>      function=function@entry=0x5612f9960b60 <__PRETTY_FUNCTION__.33659> "cpu_physical_memory_snapshot_get_dirty") at assert.c:101
> #4  cpu_physical_memory_snapshot_get_dirty (snap=snap@entry=0x5613087bdcb0,
>      start=<optimized out>, length=<optimized out>) at /usr/src/debug/qemu-2.10.0/exec.c:1198
> #5  memory_region_snapshot_get_dirty (mr=mr@entry=0x5613092e08f0,
>      snap=snap@entry=0x5613087bdcb0, addr=<optimized out>, size=<optimized out>)
>      at /usr/src/debug/qemu-2.10.0/memory.c:1949
> #6  vga_draw_graphic (full_update=0, s=0x5613092e08e0)
>      at /usr/src/debug/qemu-2.10.0/hw/display/vga.c:1678
> #7  vga_update_display (opaque=0x5613092e08e0) at /usr/src/debug/qemu-2.10.0/hw/display/vga.c:1774
> #8  vnc_refresh (dcl=0x561309116060) at ui/vnc.c:3013
> #9  dpy_refresh (s=0x5613088f8e10) at ui/console.c:1613
> #10 gui_update (opaque=0x5613088f8e10) at ui/console.c:201
> #11 timerlist_run_timers (timer_list=0x5612fba05340) at util/qemu-timer.c:536
> #12 qemu_clock_run_timers (type=<optimized out>) at util/qemu-timer.c:547
> #13 qemu_clock_run_all_timers () at util/qemu-timer.c:662
> #14 main_loop_wait (nonblocking=nonblocking@entry=0) at util/main-loop.c:521
> #15 main_loop () at vl.c:1937
> #16 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:4829
> 
> 
> It's an assertion "assert(start + length <= snap->end);" in cpu_physical_memory_snapshot_get_dirty,
> and seems that start = snap->end, when length is 511.
> 
> Digging in, I found, that in vga_draw_graphic we most probably have s->get_bpp() returning zero, which leads
> to region_end = region_start, i.e. region of zero length, which then leads to snap->end == snap->start == s->vram.ram_block->offset.
> 
> On the other hand
> 
> page0 = addr & s->vbe_size_mask = 0
> page1 = (addr + bwidth - 1) & s->vbe_size_mask = 511
> 
> which finally leads to assert fail in cpu_physical_memory_snapshot_get_dirty()
> 
> (addr is 0, bwidth is 512, width is 1024 and height is 768)..
> 
> 
> I don't understand, what get_bpp = 0 means? For me it looks strange (it is bits per pixel, isn't it?)..
> Hope you can shed some light on this.
> 
> 
> =============== some additional debugging information if it helps ========
> 
> (gdb) fr 4
> #4  0x00005612f95d7a11 in cpu_physical_memory_snapshot_get_dirty (snap=snap@entry=0x5613087bdcb0,
>      start=<optimized out>, length=<optimized out>) at /usr/src/debug/qemu-2.10.0/exec.c:1198
> 1198        assert(start + length <= snap->end);
> (gdb) list
> 1193                                                ram_addr_t length)
> 1194    {
> 1195        unsigned long page, end;
> 1196
> 1197        assert(start >= snap->start);
> 1198        assert(start + length <= snap->end);
> 1199
> 1200        end = TARGET_PAGE_ALIGN(start + length - snap->start) >> TARGET_PAGE_BITS;
> 1201        page = (start - snap->start) >> TARGET_PAGE_BITS;
> 1202
> (gdb) info locals
> page = <optimized out>
> end = <optimized out>
> __PRETTY_FUNCTION__ = "cpu_physical_memory_snapshot_get_dirty"
> (gdb) p snap
> $61 = (DirtyBitmapSnapshot *) 0x5613087bdcb0
> (gdb) p *snap
> $62 = {start = 77310459904, end = 77310459904, dirty = 0x5613087bdcc0}
> 
> 
> (gdb) fr 5
> #5  0x00005612f9621d2e in memory_region_snapshot_get_dirty (mr=mr@entry=0x5613092e08f0,
>      snap=snap@entry=0x5613087bdcb0, addr=<optimized out>, size=<optimized out>)
>      at /usr/src/debug/qemu-2.10.0/memory.c:1949
> 1949        return cpu_physical_memory_snapshot_get_dirty(snap,
> (gdb) list
> 1944
> 1945    bool memory_region_snapshot_get_dirty(MemoryRegion *mr, DirtyBitmapSnapshot *snap,
> 1946                                          hwaddr addr, hwaddr size)
> 1947    {
> 1948        assert(mr->ram_block);
> 1949        return cpu_physical_memory_snapshot_get_dirty(snap,
> 1950                    memory_region_get_ram_addr(mr) + addr, size);
> 1951    }
> 1952
> 1953    void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
> (gdb) info locals
> __PRETTY_FUNCTION__ = "memory_region_snapshot_get_dirty"
> (gdb) p *mr->ram_block
> $63 = {rcu = {next = 0x0, func = 0x0}, mr = 0x5613092e08f0,
>    host = 0x7eeef9000000 <Address 0x7eeef9000000 out of bounds>, offset = 77310459904,
>    used_length = 33554432, max_length = 33554432, resized = 0x0, flags = 0,
>    idstr = "vga.vram", '\000' <repeats 247 times>, next = {le_next = 0x5613075fca80,
>      le_prev = 0x5612fba2c748}, ramblock_notifiers = {lh_first = 0x0}, fd = -1, page_size = 4096,
>    bmap = 0x0, unsentmap = 0x0}
> 
> 
> (gdb) fr 6
> #6  0x00005612f9644af5 in vga_draw_graphic (full_update=0, s=0x5613092e08e0)
>      at /usr/src/debug/qemu-2.10.0/hw/display/vga.c:1678
> 1678                update = memory_region_snapshot_get_dirty(&s->vram, snap,
> (gdb) list
> 1673                update = memory_region_snapshot_get_dirty(&s->vram, snap,
> 1674                                                          page0, 0);
> 1675                update |= memory_region_snapshot_get_dirty(&s->vram, snap,
> 1676                                                           page1, 0);
> 1677            } else {
> 1678                update = memory_region_snapshot_get_dirty(&s->vram, snap,
> 1679                                                          page0, page1 - page0);
> 1680            }
> 1681            /* explicit invalidation for the hardware cursor (cirrus only) */
> 1682            update |= vga_scanline_invalidated(s, y);
> (gdb) info locals
> double_scan = <optimized out>
> width = 1024
> multi_scan = 0
> y = 0
> bits = <optimized out>
> snap = 0x5613087bdcb0
> byteswap = <optimized out>
> region_end = <optimized out>
> addr1 = 0
> addr = 0
> share_surface = <optimized out>
> format = <optimized out>
> y1 = 0
> y_start = -1
> multi_run = 0
> surface = 0x561308ac5c80
> update = 0
> depth = <optimized out>
> height = 768
> bwidth = 512
> region_start = <optimized out>
> vga_draw_line = 0x5612f96418d0 <vga_draw_line8d2>
> mask = <optimized out>
> page0 = <optimized out>
> d = 0x56130a68c000 ""
> v = <optimized out>
> force_shadow = false
> shift_control = <optimized out>
> page1 = <optimized out>
> disp_width = 1024
> 
> (gdb) p full_update
> $64 = 0
> (gdb) p s->line_offset
> $65 = 0
> (gdb) p s->get_bpp
> $66 = (int (*)(struct VGACommonState *)) 0x5612f9641f60 <vga_get_bpp>
> (gdb) p s->vbe_regs
> $67 = {45248, 1024, 768, 32, 0, 0, 1024, 8192, 0, 0}  // which means that vbe_enabled should return 0 and then vga_get_bpp should return 0
> (gdb) p (s->gr[5] >> 5) & 3
> $68 = 2              // it should be shift_control
> 
> 


-- 
Best regards,
Vladimir

Re: [Qemu-devel] qemu vga crash

[kraxel]

On Tue, May 21, 2019 at 01:52:31PM +0000, Vladimir Sementsov-Ogievskiy wrote:
> Could anybody help?

How about doing your homework properly?

> > Hi Gerd!
> > 
> > Writing to you, as you were the last one who committed to vga_draw_graphic, hope you can help.
> > 
> > We faced the following crash in 2.12-based Qemu, but code seems not really changed:

Pretty lame excuse for not testing a more recent release or git master.

And you are wrong.  The code *has* changed,
and the bug has been fixed a year ago already.

commit a89fe6c329799e47aaa1663650f076b28808e186
Author: Gerd Hoffmann <kraxel@redhat.com>
Date:   Mon May 14 12:31:17 2018 +0200

    vga: catch depth 0
    
    depth == 0 is used to indicate 256 color modes.  Our region calculation
    goes wrong in that case.  So detect that and just take the safe code
    path we already have for the wraparound case.
    
    While being at it also catch depth == 15 (where our region size
    calculation goes wrong too).  And make the comment more verbose,
    explaining what is going on here.
    
    Without this windows guest install might trigger an assert due to trying
    to check dirty bitmap outside the snapshot region.
    
    Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1575541
    Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
    Message-id: 20180514103117.21059-1-kraxel@redhat.com

cheers,
  Gerd

Re: [Qemu-devel] qemu vga crash

[Vladimir Sementsov-Ogievskiy]

21.05.2019 18:53, kraxel@redhat.com wrote:
> On Tue, May 21, 2019 at 01:52:31PM +0000, Vladimir Sementsov-Ogievskiy wrote:
>> Could anybody help?
> 
> How about doing your homework properly?
> 
>>> Hi Gerd!
>>>
>>> Writing to you, as you were the last one who committed to vga_draw_graphic, hope you can help.
>>>
>>> We faced the following crash in 2.12-based Qemu, but code seems not really changed:
> 
> Pretty lame excuse for not testing a more recent release or git master.
> 
> And you are wrong.  The code *has* changed,
> and the bug has been fixed a year ago already.

Oops, seems like I just compared wrong versions when found no real difference, as bug actually from 2.10,
and fixed in 2.12.

Thank you and sorry for the noise.

> 
> commit a89fe6c329799e47aaa1663650f076b28808e186
> Author: Gerd Hoffmann <kraxel@redhat.com>
> Date:   Mon May 14 12:31:17 2018 +0200
> 
>      vga: catch depth 0
>      
>      depth == 0 is used to indicate 256 color modes.  Our region calculation
>      goes wrong in that case.  So detect that and just take the safe code
>      path we already have for the wraparound case.
>      
>      While being at it also catch depth == 15 (where our region size
>      calculation goes wrong too).  And make the comment more verbose,
>      explaining what is going on here.
>      
>      Without this windows guest install might trigger an assert due to trying
>      to check dirty bitmap outside the snapshot region.
>      
>      Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1575541
>      Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>      Message-id: 20180514103117.21059-1-kraxel@redhat.com
> 
> cheers,
>    Gerd
> 


-- 
Best regards,
Vladimir