Hello Marc, all
+-- On Wed, 16 Nov 2016, Marc-André Lureau wrote --+
| For completeness, do you have a reproducer and/or a backtrace?
Yes, there is.
===
Thread 4 "qemu-system-x86" received signal SIGFPE, Arithmetic exception.
[Switching to Thread 0x7ffff002c700 (LWP 10506)]
0x000055555599fe2e in cirrus_do_copy (s=0x55555758af60, dst=0, src=0, w=2048,
h=4096) at hw/display/cirrus_vga.c:735
735 sx = (src % ABS(s->cirrus_blt_srcpitch)) / depth;
(gdb) bt
#0 0x000055555599fe2e in cirrus_do_copy (s=0x55555758af60, dst=0, src=0,
w=2048, h=4096) at hw/display/cirrus_vga.c:735
#1 0x00005555559a0134 in cirrus_bitblt_videotovideo_copy (s=0x55555758af60) at
hw/display/cirrus_vga.c:793
#2 0x00005555559a0609 in cirrus_bitblt_videotovideo (s=0x55555758af60) at
hw/display/cirrus_vga.c:915
#3 0x00005555559a0d77 in cirrus_bitblt_start (s=0x55555758af60) at
hw/display/cirrus_vga.c:1056
#4 0x00005555559a1ad3 in cirrus_vga_write_gr (s=0x55555758af60, reg_index=42,
reg_value=0) at hw/display/cirrus_vga.c:1572
#5 0x00005555559a3ad8 in cirrus_vga_ioport_write (opaque=0x55555758af60,
addr=975, val=0, size=1) at hw/display/cirrus_vga.c:2678
#6 0x00005555557a8df7 in memory_region_write_accessor (mr=0x55555759ba50,
addr=31, ...
#7 0x00005555557a900f in access_with_adjusted_size (addr=31,
value=0x7ffff002b8b8, ...
#8 0x00005555557ab74f in memory_region_dispatch_write (mr=0x55555759ba50,
addr=31, ...
#9 0x0000555555757003 in address_space_write_continue (as=0x55555621b5a0
<address_space_io>, ...
#10 0x000055555575714b in address_space_write (as=0x55555621b5a0
<address_space_io>, ...
#11 0x00005555557574d7 in address_space_rw (as=0x55555621b5a0
<address_space_io>, ...
#12 0x00005555557a53d1 in kvm_handle_io (port=975, attrs=...,
data=0x7ffff7ff0000, ...
#13 0x00005555557a58d7 in kvm_cpu_exec (cpu=0x555556746f90)
#14 0x000055555578c752 in qemu_kvm_cpu_thread_fn (arg=0x555556746f90)
#15 0x00007ffff5e8d5ca in start_thread () from /lib64/libpthread.so.0
#16 0x00007ffff5bc70ed in clone () from /lib64/libc.so.6
===
| > --- a/hw/display/cirrus_vga.c
| > +++ b/hw/display/cirrus_vga.c
| > @@ -272,6 +272,9 @@ static void cirrus_update_memory_access(CirrusVGAState
| > *s);
| > static bool blit_region_is_unsafe(struct CirrusVGAState *s,
| > int32_t pitch, int32_t addr)
| > {
| > + if (!pitch) {
| > + return true;
| > + }
| >
|
| That doesn't look directly related to 'cirrus_get_bpp', care to explain?
'blit_region_is_unsafe' is called from 'blit_is_unsafe' to check if blit
parameters (cirrus_blt_srcpitch/cirrus_blt_dstpitch) are safe for
'cirrus_do_copy'. These too could lead to div by zero in cirrus_do_copy
static int cirrus_do_copy(CirrusVGAState *s, ...)
{
...
sx = (src % ABS(s->cirrus_blt_srcpitch)) / depth;
sy = (src / ABS(s->cirrus_blt_srcpitch));
dx = (dst % ABS(s->cirrus_blt_dstpitch)) / depth;
dy = (dst / ABS(s->cirrus_blt_dstpitch));
}
| btw, not directly related to your patch, but the code looks strange in
| cirrus_bitblt_videotovideo(), cirrus_bitblt_reset() is called if(ret), and
| later if (!ret) in cirrus_bitblt_start(), that looks a bit weird, but it
| may be fine.
I think that is to avoid calling 'cirrus_bitblt_reset' twice.
Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
