qemu-commits
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-commits] [qemu/qemu] 215902: fix :cirrus_vga fix OOB read case qem


From: GitHub
Subject: [Qemu-commits] [qemu/qemu] 215902: fix :cirrus_vga fix OOB read case qemu Segmentatio...
Date: Thu, 16 Mar 2017 11:00:12 -0700

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 215902d7b6fb50c6fc216fc74f770858278ed904
      
https://github.com/qemu/qemu/commit/215902d7b6fb50c6fc216fc74f770858278ed904
  Author: hangaohuai <address@hidden>
  Date:   2017-03-16 (Thu, 16 Mar 2017)

  Changed paths:
    M hw/display/cirrus_vga_rop.h

  Log Message:
  -----------
  fix :cirrus_vga fix OOB read case qemu Segmentation fault

check the validity of parameters in cirrus_bitblt_rop_fwd_transp_xxx
and cirrus_bitblt_rop_fwd_xxx to avoid the OOB read which causes qemu 
Segmentation fault.

After the fix, we will touch the assert in
cirrus_invalidate_region:
assert(off_cur_end >= off_cur);

Signed-off-by: fangying <address@hidden>
Signed-off-by: hangaohuai <address@hidden>
Message-id: address@hidden
Signed-off-by: Gerd Hoffmann <address@hidden>


  Commit: 50628d3479e4f9aa97e323506856e394fe7ad7a6
      
https://github.com/qemu/qemu/commit/50628d3479e4f9aa97e323506856e394fe7ad7a6
  Author: Gerd Hoffmann <address@hidden>
  Date:   2017-03-16 (Thu, 16 Mar 2017)

  Changed paths:
    M hw/display/cirrus_vga.c
    M include/ui/console.h
    M ui/console.c
    M ui/vnc.c

  Log Message:
  -----------
  cirrus/vnc: zap bitblit support from console code.

There is a special code path (dpy_gfx_copy) to allow graphic emulation
notify user interface code about bitblit operations carryed out by
guests.  It is supported by cirrus and vnc server.  The intended purpose
is to optimize display scrolls and just send over the scroll op instead
of a full display update.

This is rarely used these days though because modern guests simply don't
use the cirrus blitter any more.  Any linux guest using the cirrus drm
driver doesn't.  Any windows guest newer than winxp doesn't ship with a
cirrus driver any more and thus uses the cirrus as simple framebuffer.

So this code tends to bitrot and bugs can go unnoticed for a long time.
See for example commit "3e10c3e vnc: fix qemu crash because of SIGSEGV"
which fixes a bug lingering in the code for almost a year, added by
commit "c7628bf vnc: only alloc server surface with clients connected".

Also the vnc server will throttle the frame rate in case it figures the
network can't keep up (send buffers are full).  This doesn't work with
dpy_gfx_copy, for any copy operation sent to the vnc client we have to
send all outstanding updates beforehand, otherwise the vnc client might
run the client side blit on outdated data and thereby corrupt the
display.  So this dpy_gfx_copy "optimization" might even make things
worse on slow network links.

Lets kill it once for all.

Oh, and one more reason: Turns out (after writing the patch) we have a
security bug in that code path ...

Fixes: CVE-2016-9603
Signed-off-by: Gerd Hoffmann <address@hidden>
Message-id: address@hidden


  Commit: 73c148130b58709f0f2abfedbae92681d87eb404
      
https://github.com/qemu/qemu/commit/73c148130b58709f0f2abfedbae92681d87eb404
  Author: Gerd Hoffmann <address@hidden>
  Date:   2017-03-16 (Thu, 16 Mar 2017)

  Changed paths:
    M hw/display/cirrus_vga.c
    M include/hw/compat.h

  Log Message:
  -----------
  cirrus: switch to 4 MB video memory by default

Quoting cirrus source code:
   Follow real hardware, cirrus card emulated has 4 MB video memory.
   Also accept 8 MB/16 MB for backward compatibility.

So just use 4MB by default.  We decided to leave that at 8MB by default
a while ago, for live migration compatibility reasons.  But we have
compat properties to handle that, so that isn't a compeling reason.

This also removes some sanity check inconsistencies in the cirrus code.
Some places check against the allocated video memory, some places check
against the 4MB physical hardware has.  Guest code can trigger asserts
because of that.

Signed-off-by: Gerd Hoffmann <address@hidden>
Message-id: address@hidden


  Commit: 827bd5172641f2a360ff9a3bad57bcf82e7f03f0
      
https://github.com/qemu/qemu/commit/827bd5172641f2a360ff9a3bad57bcf82e7f03f0
  Author: Gerd Hoffmann <address@hidden>
  Date:   2017-03-16 (Thu, 16 Mar 2017)

  Changed paths:
    M hw/display/cirrus_vga.c

  Log Message:
  -----------
  cirrus: add option to disable blitter

Ok, we have this beast in the cirrus code which is not used at all by
modern guests, except when you try to find security holes in qemu.  So,
add an option to disable blitter altogether.  Guests released within
the last ten years should not show any rendering issues if you turn off
blitter support.

There are no known bugs in the cirrus blitter code.  But in the past we
hoped a few times already that we've finally nailed the last issue.  So
having some easy way to mitigate in case yet another blitter issue shows
up certainly makes me sleep a bit better at night.

For completeness:  The by far better way to mitigate is to switch away
from cirrus and use stdvga instead.  Or something more modern like
virtio-vga in case your guest has support for it.

Signed-off-by: Gerd Hoffmann <address@hidden>
Message-id: address@hidden


  Commit: e048dac616748273c2153490e9fdf1da242f0cad
      
https://github.com/qemu/qemu/commit/e048dac616748273c2153490e9fdf1da242f0cad
  Author: Gerd Hoffmann <address@hidden>
  Date:   2017-03-16 (Thu, 16 Mar 2017)

  Changed paths:
    M hw/display/cirrus_vga.c

  Log Message:
  -----------
  cirrus: fix cirrus_invalidate_region

off_cur_end is exclusive, so off_cur_end == cirrus_addr_mask is valid.
Fix calculation to make sure to allow that, otherwise the assert added
by commit f153b563f8cf121aebf5a2fff5f0110faf58ccb3 can trigger for valid
blits.

Test case: boot windows nt 4.0

Signed-off-by: Gerd Hoffmann <address@hidden>
Message-id: address@hidden


  Commit: 026aeffcb4752054830ba203020ed6eb05bcaba8
      
https://github.com/qemu/qemu/commit/026aeffcb4752054830ba203020ed6eb05bcaba8
  Author: Gerd Hoffmann <address@hidden>
  Date:   2017-03-16 (Thu, 16 Mar 2017)

  Changed paths:
    M hw/display/cirrus_vga.c
    M hw/display/cirrus_vga_rop.h
    M hw/display/cirrus_vga_rop2.h

  Log Message:
  -----------
  cirrus: stop passing around dst pointers in the blitter

Instead pass around the address (aka offset into vga memory).  Calculate
the pointer in the rop_* functions, after applying the mask to the
address, to make sure the address stays within the valid range.

Signed-off-by: Gerd Hoffmann <address@hidden>
Message-id: address@hidden


  Commit: ffaf857778286ca54e3804432a2369a279e73aa7
      
https://github.com/qemu/qemu/commit/ffaf857778286ca54e3804432a2369a279e73aa7
  Author: Gerd Hoffmann <address@hidden>
  Date:   2017-03-16 (Thu, 16 Mar 2017)

  Changed paths:
    M hw/display/cirrus_vga.c
    M hw/display/cirrus_vga_rop.h
    M hw/display/cirrus_vga_rop2.h

  Log Message:
  -----------
  cirrus: stop passing around src pointers in the blitter

Does basically the same as "cirrus: stop passing around dst pointers in
the blitter", just for the src pointer instead of the dst pointer.

For the src we have to care about cputovideo blits though and fetch the
data from s->cirrus_bltbuf instead of vga memory.  The cirrus_src*()
helper functions handle that.

Signed-off-by: Gerd Hoffmann <address@hidden>
Message-id: address@hidden


  Commit: 272d7dee5951f926fad1911f2f072e5915cdcba0
      
https://github.com/qemu/qemu/commit/272d7dee5951f926fad1911f2f072e5915cdcba0
  Author: Peter Maydell <address@hidden>
  Date:   2017-03-16 (Thu, 16 Mar 2017)

  Changed paths:
    M hw/display/cirrus_vga.c
    M hw/display/cirrus_vga_rop.h
    M hw/display/cirrus_vga_rop2.h
    M include/hw/compat.h
    M include/ui/console.h
    M ui/console.c
    M ui/vnc.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/kraxel/tags/pull-cirrus-20170316-1' 
into staging

cirrus: blitter fixes.

# gpg: Signature made Thu 16 Mar 2017 09:05:22 GMT
# gpg:                using RSA key 0x4CB6D8EED3E87138
# gpg: Good signature from "Gerd Hoffmann (work) <address@hidden>"
# gpg:                 aka "Gerd Hoffmann <address@hidden>"
# gpg:                 aka "Gerd Hoffmann (private) <address@hidden>"
# Primary key fingerprint: A032 8CFF B93A 17A7 9901  FE7D 4CB6 D8EE D3E8 7138

* remotes/kraxel/tags/pull-cirrus-20170316-1:
  cirrus: stop passing around src pointers in the blitter
  cirrus: stop passing around dst pointers in the blitter
  cirrus: fix cirrus_invalidate_region
  cirrus: add option to disable blitter
  cirrus: switch to 4 MB video memory by default
  cirrus/vnc: zap bitblit support from console code.
  fix :cirrus_vga fix OOB read case qemu Segmentation fault

# Conflicts:
#       include/hw/compat.h

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/c5e737e5fbb8...272d7dee5951

reply via email to

[Prev in Thread] Current Thread [Next in Thread]