]> xenbits.xensource.com Git - qemu-xen-traditional.git/log
qemu-xen-traditional.git
8 years agocirrus/vnc: zap drop bitblit support from console code. stable-4.8 xen-4.8.1 xen-4.8.2 xen-4.8.3 xen-4.8.4 xen-4.8.5
Gerd Hoffmann [Thu, 9 Mar 2017 11:14:55 +0000 (11:14 +0000)]
cirrus/vnc: zap drop 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.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
These changes (dropping dpy_copy and all its references and
implementations) reimplemented for qemu-xen-traditional.

This is XSA-211.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 8051789e982499050680a26febeada7467e18a8d)

8 years agocirrus: add blit_is_unsafe call to cirrus_bitblt_cputovideo
Gerd Hoffmann [Wed, 22 Feb 2017 16:25:19 +0000 (16:25 +0000)]
cirrus: add blit_is_unsafe call to cirrus_bitblt_cputovideo

CIRRUS_BLTMODE_MEMSYSSRC blits do NOT check blit destination
and blit width, at all.  Oops.  Fix it.

Security impact: high.

The missing blit destination check allows to write to host memory.
Basically same as CVE-2014-8106 for the other blit variants.

The missing blit width check allows to overflow cirrus_bltbuf,
with the attractive target cirrus_srcptr (current cirrus_bltbuf write
position) being located right after cirrus_bltbuf in CirrusVGAState.

Due to cirrus emulation writing cirrus_bltbuf bytewise the attacker
hasn't full control over cirrus_srcptr though, only one byte can be
changed.  Once the first byte has been modified further writes land
elsewhere.

[ This is CVE-2017-2620 / XSA-209  - Ian Jackson ]

Fixed compilation by removing extra parameter to blit_is_unsafe. -iwj

Reported-by: Gerd Hoffmann <ghoffman@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 83256b312acfddda34d96e9bba30e93387481091)

8 years agocirrus: fix oob access issue (CVE-2017-2615)
Li Qiang [Mon, 13 Feb 2017 15:22:15 +0000 (15:22 +0000)]
cirrus: fix oob access issue (CVE-2017-2615)

When doing bitblt copy in backward mode, we should minus the
blt width first just like the adding in the forward mode. This
can avoid the oob access of the front of vga's vram.

This is XSA-208.

upstream-commit-id: 62d4c6bd5263bb8413a06c80144fc678df6dfb64

Signed-off-by: Li Qiang <liqiang6-s@360.cn>
{ kraxel: with backward blits (negative pitch) addr is the topmost
          address, so check it as-is against vram size ]

[ This is CVE-2017-2615 / XSA-208  - Ian Jackson ]

Cc: qemu-stable@nongnu.org
Cc: P J P <ppandit@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Wolfgang Bumiller <w.bumiller@proxmox.com>
Fixes: d3532a0db02296e687711b8cdc7791924efccea0 (CVE-2014-8106)
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1485938101-26602-1-git-send-email-kraxel@redhat.com
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit c4018bc4d638918b3f8fb49dd3b379abb5658ee1)

8 years agoqemu: ioport_read, ioport_write: be defensive about 32-bit addresses
Ian Jackson [Mon, 14 Nov 2016 17:19:46 +0000 (17:19 +0000)]
qemu: ioport_read, ioport_write: be defensive about 32-bit addresses

On x86, ioport addresses are 16-bit.  That these functions take 32-bit
arguments is a mistake.  Changing the argument type to 16-bit will
discard the top bits of any erroneous values from elsewhere in qemu.

Also, check just before use that the value is in range.  (This turns
an ill-advised change to MAX_IOPORTS into a possible guest crash
rather than a privilege escalation vulnerability.)

And, in the Xen ioreq processor, clamp incoming ioport addresses to
16-bit values.  Xen will never write >16-bit values but the guest may
have access to the ioreq ring.  We want to defend the rest of the qemu
code from wrong values.

This is XSA-199.

Reported-by: yanghongke <yanghongke@huawei.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
(cherry picked from commit b669e922b37b8957248798a5eb7aa96a666cd3fe)

8 years agoxen: fix ioreq handling xen-4.8.0 xen-4.8.0-rc7
Jan Beulich [Tue, 29 Nov 2016 16:36:38 +0000 (16:36 +0000)]
xen: fix ioreq handling

Avoid double fetches and bounds check size to avoid overflowing
internal variables.

This is XSA-197.

Reported-by: yanghongke <yanghongke@huawei.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
8 years agovirtio: error out if guest exceeds virtqueue size xen-4.8.0-rc1
P J P [Tue, 26 Jul 2016 14:31:59 +0000 (15:31 +0100)]
virtio: error out if guest exceeds virtqueue size

A broken or malicious guest can submit more requests than the virtqueue
size permits.

The guest can submit requests without bothering to wait for completion
and is therefore not bound by virtqueue size.  This requires reusing
vring descriptors in more than one request, which is incorrect but
possible.  Processing a request allocates a VirtQueueElement and
therefore causes unbounded memory allocation controlled by the guest.

Exit with an error if the guest provides more requests than the
virtqueue size permits.  This bounds memory allocation and makes the
buggy guest visible to the user.

Reported-by: Zhenhao Hong <zhenhaohong@gmail.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
8 years agomain loop: Big hammer to fix logfile disk DoS in Xen setups
Ian Jackson [Thu, 19 May 2016 18:38:35 +0000 (19:38 +0100)]
main loop: Big hammer to fix logfile disk DoS in Xen setups

Each time round the main loop, we now fstat stderr.  If it is too big,
we dup2 /dev/null onto it.  This is not a very pretty patch but it is
very simple, easy to see that it's correct, and has a low risk of
collateral damage.

There is no limit by default but can be adjusted by setting a new
environment variable.

This is part of CVE-2014-3672.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Tested-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Set the default to 0 so that it won't affect non-xen installation. The
limit will be set by Xen toolstack.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
(ad-hoc cherry-pick from 44a072f0de0d57c95c2212bbce02888832b7b74f)
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
8 years agoFix build with newer version of GNUTLS
Wei Liu [Thu, 5 May 2016 10:14:44 +0000 (11:14 +0100)]
Fix build with newer version of GNUTLS

gnutls_kx_set_priority, gnutls_certificate_type_set_priority and
gnutls_protocol_set_priority were deprecated and eventually removed in
GNUTLS 3.4. Application should use gnutls_priority_set_direct instead
per [0].

gnutls_anon_server_credentials was deprecated at some point. Application
should use gnutls_anon_server_credentials_t instead.

Provide compatibility layer for QEMU traditional. This commit is in fact
backport of two upstream QEMU commits:
1. f40d55081667a716312b9a8b6e13835c4074f56b
2. 7d2a929feba319c18603e324b1750830d6c8b7a1

[0] https://www.gnutls.org/manual/html_node/Upgrading-from-previous-versions.html

Signed-off-by: Sjoer van der Ploeg <sfjuocekr@gmail.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
8 years agortl8139: check TCP Data Offset field
Stefan Hajnoczi [Wed, 15 Jul 2015 17:17:04 +0000 (18:17 +0100)]
rtl8139: check TCP Data Offset field

The TCP Data Offset field contains the length of the header.  Make sure
it is valid and does not exceed the IP data length.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
[Backport to qemu-xen-tradition]
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
8 years agortl8139: skip offload on short TCP header
Stefan Hajnoczi [Wed, 15 Jul 2015 17:17:03 +0000 (18:17 +0100)]
rtl8139: skip offload on short TCP header

TCP Large Segment Offload accesses the TCP header in the packet.  If the
packet is too short we must not attempt to access header fields:

  tcp_header *p_tcp_hdr = (tcp_header*)(eth_payload_data + hlen);
  int tcp_hlen = TCP_HEADER_DATA_OFFSET(p_tcp_hdr);

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
[Backport to qemu-xen-tradition]
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
8 years agortl8139: check IP Total Length field
Stefan Hajnoczi [Wed, 15 Jul 2015 17:17:02 +0000 (18:17 +0100)]
rtl8139: check IP Total Length field

The IP Total Length field includes the IP header and data.  Make sure it
is valid and does not exceed the Ethernet payload size.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
[Backport to qemu-xen-tradition]
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
8 years agortl8139: check IP Header Length field
Stefan Hajnoczi [Wed, 15 Jul 2015 17:17:01 +0000 (18:17 +0100)]
rtl8139: check IP Header Length field

The IP Header Length field was only checked in the IP checksum case, but
is used in other cases too.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
[Backport to qemu-xen-tradition]
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
8 years agortl8139: skip offload on short Ethernet/IP header
Stefan Hajnoczi [Wed, 15 Jul 2015 17:17:00 +0000 (18:17 +0100)]
rtl8139: skip offload on short Ethernet/IP header

Transmit offload features access Ethernet and IP headers the packet.  If
the packet is too short we must not attempt to access header fields:

  int proto = be16_to_cpu(*(uint16_t *)(saved_buffer + 12));
  ...
  eth_payload_data = saved_buffer + ETH_HLEN;
  ...
  ip = (ip_header*)eth_payload_data;
  if (IP_HEADER_VERSION(ip) != IP_HEADER_VERSION_4) {

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
[Backport to qemu-xen-tradition]
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
8 years agortl8139: drop tautologous if (ip) {...} statement
Stefan Hajnoczi [Wed, 15 Jul 2015 17:16:59 +0000 (18:16 +0100)]
rtl8139: drop tautologous if (ip) {...} statement

The previous patch stopped using the ip pointer as an indicator that the
IP header is present.  When we reach the if (ip) {...} statement we know
ip is always non-NULL.

Remove the if statement to reduce nesting.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
[Backport to qemu-xen-tradition]
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
8 years agortl8139: avoid nested ifs in IP header parsing
Stefan Hajnoczi [Wed, 15 Jul 2015 17:16:58 +0000 (18:16 +0100)]
rtl8139: avoid nested ifs in IP header parsing

Transmit offload needs to parse packet headers.  If header fields have
unexpected values the offload processing is skipped.

The code currently uses nested ifs because there is relatively little
input validation.  The next patches will add missing input validation
and a goto label is more appropriate to avoid deep if statement nesting.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
[Backport to qemu-xen-tradition]
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
8 years agovga: make sure vga register setup for vbe stays intact (CVE-2016-3712).
Gerd Hoffmann [Tue, 26 Apr 2016 12:48:06 +0000 (14:48 +0200)]
vga: make sure vga register setup for vbe stays intact (CVE-2016-3712).

Call vbe_update_vgaregs() when the guest touches GFX, SEQ or CRT
registers, to make sure the vga registers will always have the
values needed by vbe mode.  This makes sure the sanity checks
applied by vbe_fixup_regs() are effective.

Without this guests can muck with shift_control, can turn on planar
vga modes or text mode emulation while VBE is active, making qemu
take code paths meant for CGA compatibility, but with the very
large display widths and heigts settable using VBE registers.

Which is good for one or another buffer overflow.  Not that
critical as they typically read overflows happening somewhere
in the display code.  So guests can DoS by crashing qemu with a
segfault, but it is probably not possible to break out of the VM.

Fixes: CVE-2016-3712
Reported-by: Zuozhi Fzz <zuozhi.fzz@alibaba-inc.com>
Reported-by: P J P <ppandit@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
[Backport to qemu-xen-tradition]
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
8 years agovga: update vga register setup on vbe changes
Gerd Hoffmann [Tue, 26 Apr 2016 13:39:22 +0000 (15:39 +0200)]
vga: update vga register setup on vbe changes

Call the new vbe_update_vgaregs() function on vbe configuration
changes, to make sure vga registers are up-to-date.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
[Backport to qemu-xen-tradition]
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
8 years agovga: factor out vga register setup
Gerd Hoffmann [Tue, 26 Apr 2016 13:24:18 +0000 (15:24 +0200)]
vga: factor out vga register setup

When enabling vbe mode qemu will setup a bunch of vga registers to make
sure the vga emulation operates in correct mode for a linear
framebuffer.  Move that code to a separate function so we can call it
from other places too.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
[Backport to qemu-xen-tradition]
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
8 years agovga: add vbe_enabled() helper
Gerd Hoffmann [Tue, 26 Apr 2016 12:11:34 +0000 (14:11 +0200)]
vga: add vbe_enabled() helper

Makes code a bit easier to read.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
[Backport to qemu-xen-tradition]
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
8 years agovga: fix banked access bounds checking (CVE-2016-3710)
Gerd Hoffmann [Tue, 26 Apr 2016 06:49:10 +0000 (08:49 +0200)]
vga: fix banked access bounds checking (CVE-2016-3710)

vga allows banked access to video memory using the window at 0xa00000
and it supports a different access modes with different address
calculations.

The VBE bochs extentions support banked access too, using the
VBE_DISPI_INDEX_BANK register.  The code tries to take the different
address calculations into account and applies different limits to
VBE_DISPI_INDEX_BANK depending on the current access mode.

Which is probably effective in stopping misprogramming by accident.
But from a security point of view completely useless as an attacker
can easily change access modes after setting the bank register.

Drop the bogus check, add range checks to vga_mem_{readb,writeb}
instead.

Fixes: CVE-2016-3710
Reported-by: Qinghao Tang <luodalongde@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
[Backport to qemu-xen-tradition]
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agoqemu-xen-traditional: Add libxenforeignmemory to rpath-link xen-4.7.0-rc1
Ian Campbell [Fri, 15 Jan 2016 13:23:56 +0000 (13:23 +0000)]
qemu-xen-traditional: Add libxenforeignmemory to rpath-link

libxenctrl links against this library.

Also, request the compat xc_map_foreign API from libxc.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
9 years agoqemu-xen-traditional: Add libxencall to rpath-link
Ian Campbell [Fri, 15 Jan 2016 13:23:55 +0000 (13:23 +0000)]
qemu-xen-traditional: Add libxencall to rpath-link

libxenctrl links against this library

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
9 years agoqemu-xen-traditional: Use libxengnttab
Ian Campbell [Fri, 15 Jan 2016 13:23:54 +0000 (13:23 +0000)]
qemu-xen-traditional: Use libxengnttab

/dev/xen/gntdev related wrappers have been moved out of libxenctrl
into their own library.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
9 years agoqemu-xen-traditional: Use libxenevtchn
Ian Campbell [Fri, 15 Jan 2016 13:23:53 +0000 (13:23 +0000)]
qemu-xen-traditional: Use libxenevtchn

/dev/xen/evtchn related wrappers have been moved out of libxenctrl
into their own library.

Note that i386-dm/helper2.c's xc_interface * was always really an
xc_evtchn *, it's just they used to be typedefs to the same thing...

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
9 years agoMSI-X: avoid array overrun upon MSI-X table writes
Jan Beulich [Mon, 4 Jan 2016 15:34:29 +0000 (15:34 +0000)]
MSI-X: avoid array overrun upon MSI-X table writes

pt_msix_init() allocates msix->msix_entry[] to just cover
msix->total_entries entries. While pci_msix_readl() resorts to reading
physical memory for out of bounds reads, pci_msix_writel() so far
simply accessed/corrupted unrelated memory.

pt_iomem_map()'s call to cpu_register_physical_memory() registers a
page granular region, which is necessary as the Pending Bit Array may
share space with the MSI-X table (but nothing else is allowed to). This
also explains why pci_msix_readl() actually honors out of bounds reads,
but pci_msi_writel() doesn't need to.

This is XSA-164.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
9 years agoblkif: Avoid double access to src->nr_segments
Stefano Stabellini [Fri, 20 Nov 2015 15:56:00 +0000 (10:56 -0500)]
blkif: Avoid double access to src->nr_segments

src is stored in shared memory and src->nr_segments is dereferenced
twice at the end of the function.  If a compiler decides to compile this
into two separate memory accesses then the size limitation could be
bypassed.

Fix it by removing the double access to src->nr_segments.

This is part of XSA-155.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
9 years agoxenfb: avoid reading twice the same fields from the shared page
Stefano Stabellini [Fri, 20 Nov 2015 15:37:08 +0000 (10:37 -0500)]
xenfb: avoid reading twice the same fields from the shared page

Reading twice the same field could give the guest an attack of
opportunity. In the case of event->type, gcc could compile the switch
statement into a jump table, effectively ending up reading the type
field multiple times.

This is part of XSA-155.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
9 years agoqemu-xen-traditional: Use xentoollog as a separate library
Ian Campbell [Thu, 3 Dec 2015 11:23:16 +0000 (11:23 +0000)]
qemu-xen-traditional: Use xentoollog as a separate library

This has been split out of libxenctrl, so the build needs to be able
to find the header and the library. QEMU does not use xtl_* itself so
-rpath-link is sufficient to allow linking to libxenctrl.so (which
links against libxentoollog).

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
9 years agonet: pcnet: add check to validate receive data size(CVE-2015-7504)
Ian Jackson [Wed, 9 Dec 2015 11:47:35 +0000 (11:47 +0000)]
net: pcnet: add check to validate receive data size(CVE-2015-7504)

In loopback mode, pcnet_receive routine appends CRC code to the
receive buffer. If the data size given is same as the buffer size,
the appended CRC code overwrites 4 bytes after s->buffer. Added a
check to avoid that.

This is XSA-162.

Reported-by: Qinghao Tang <luodalongde@gmail.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
9 years agoblock-vvfat: fix resource leaks in read_directory()
Yunlei Ding [Tue, 11 Mar 2014 10:19:23 +0000 (10:19 +0000)]
block-vvfat: fix resource leaks in read_directory()

Signed-off-by: Yunlei Ding <yunlei.ding@citrix.com>
Coverity-IDs: 1055920 1055921
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agoblock-raw-posix: Fix memory leak in posix_aio_init()
Yunlei Ding [Tue, 11 Mar 2014 10:14:56 +0000 (10:14 +0000)]
block-raw-posix: Fix memory leak in posix_aio_init()

Free allocated memory s before return.

Signed-off-by: Yunlei Ding <yunlei.ding@citrix.com>
Coverity-ID: 1055915
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agoblock-nbd: close sock in nbd_open() error path
Yunlei Ding [Tue, 11 Mar 2014 10:12:01 +0000 (10:12 +0000)]
block-nbd: close sock in nbd_open() error path

Close sock handle before return.

Signed-off-by: Yunlei Ding <yunlei.ding@citrix.com>
Coverity-ID: 1055914
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agoide: don't leak irq array in pci_cmd646_ide_init()
Yunlei Ding [Fri, 18 Apr 2014 05:11:30 +0000 (06:11 +0100)]
ide: don't leak irq array in pci_cmd646_ide_init()

Call qemu_allocate_irq() twice instead of qemu_allocate_irqs to
allocate memory.

Signed-off-by: Yunlei Ding <yunlei.ding@citrix.com>
(defects not identified by Coverity Scan)
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agonet: initialize parameters before use in net_socket_fd_init_dgram()
Yunlei Ding [Tue, 11 Mar 2014 09:55:59 +0000 (09:55 +0000)]
net: initialize parameters before use in net_socket_fd_init_dgram()

Signed-off-by: Yunlei Ding <yunlei.ding@citrix.com>
Coverity-IDs: 1005339 1005340
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agovirtio-blk: correctly link new request in virtio_blk_load()
Yunlei Ding [Tue, 11 Mar 2014 09:49:59 +0000 (09:49 +0000)]
virtio-blk: correctly link new request in virtio_blk_load()

s->rq should be set with req instead of req-next.

Signed-off-by: Yunlei Ding <yunlei.ding@citrix.com>
Coverity-ID: 1055910
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agoblock-vvfat: fix memory leak in check_directory_consistency()
Kaifeng Zhu [Fri, 7 Mar 2014 16:52:47 +0000 (16:52 +0000)]
block-vvfat: fix memory leak in check_directory_consistency()

Memory pointed by cluster leaks in error handling code.

Signed-off-by: Kaifeng Zhu <kaifeng.zhu@citrix.com>
Coverity-ID: 1055917
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agoblock-vvfat: fix memory/handle leaks in commit_one_file()
Kaifeng Zhu [Fri, 7 Mar 2014 16:44:33 +0000 (16:44 +0000)]
block-vvfat: fix memory/handle leaks in commit_one_file()

Some handles and memory in commit_one_file are going to be leaked if
certain function calls failed.

Signed-off-by: Kaifeng Zhu <kaifeng.zhu@citrix.com>
Coverity-IDs: 1055918 1055919
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agonet: Fix memory/handle leaks in net_socket_listen_init()
Kaifeng Zhu [Tue, 22 Apr 2014 06:38:24 +0000 (07:38 +0100)]
net: Fix memory/handle leaks in net_socket_listen_init()

fd and s could be leaked in case bind/listen failed.

Signed-off-by: Kaifeng Zhu <kaifeng.zhu@citrix.com>
Coverity-IDs: 1055923 1055924
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agonet: don't leak an fd after an error
Kaifeng Zhu [Fri, 7 Mar 2014 16:31:06 +0000 (16:31 +0000)]
net: don't leak an fd after an error

fd will be leaked if launch_script failed.

Signed-off-by: Kaifeng Zhu <kaifeng.zhu@citrix.com>
Coverity-ID: 1055925
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agohw/ide: fix memory leak from qemu_allocate_irqs()
Kaifeng Zhu [Fri, 7 Mar 2014 16:10:14 +0000 (16:10 +0000)]
hw/ide: fix memory leak from qemu_allocate_irqs()

qemu_allocate_irqs would return an array of irqs, not store the allocated
array pointer, and subsequently leak it.

Signed-off-by: Kaifeng Zhu <kaifeng.zhu@citrix.com>
(defects not identified by Coverity Scan)
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agoqemu-char: fix memory leak in qemu_char_open_pty()
Kaifeng Zhu [Fri, 7 Mar 2014 15:27:36 +0000 (15:27 +0000)]
qemu-char: fix memory leak in qemu_char_open_pty()

The momery pointed by s and chr could be leaked if openpty return a value
less then 0.

Signed-off-by: Kaifeng Zhu <kaifeng.zhu@citrix.com>
Coverity-IDs: 1055926 1055927
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agohw/device-hotplug: fix test of drive_add() return
Kaifeng Zhu [Fri, 7 Mar 2014 15:16:42 +0000 (15:16 +0000)]
hw/device-hotplug: fix test of drive_add() return

drive_opt_idx could be -1 in case error occurs inside drive_add, so the error
check should be "if (drive_opt_idx < 0)" instead of original
"if (!drive_opt_idx)".

Signed-off-by: Kaifeng Zhu <kaifeng.zhu@citrix.com>
Coverity-ID: 1055574
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agoconsole: Avoid overrunning the dmask arrays
Kaifeng Zhu [Fri, 7 Mar 2014 15:11:21 +0000 (15:11 +0000)]
console: Avoid overrunning the dmask arrays

The valide range of font_data should be [0, 0xFF].

Signed-off-by: Kaifeng Zhu <kaifeng.zhu@citrix.com>
(defects not identified by Coverity Scan)
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agoblock-cow: don't close cow_fd twice on error
Kaifeng Zhu [Fri, 7 Mar 2014 15:03:40 +0000 (15:03 +0000)]
block-cow: don't close cow_fd twice on error

Signed-off-by: Kaifeng Zhu <kaifeng.zhu@citrix.com>
Coverity-ID: 1056200
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agoreadline: fix memory corruption when adding history
Kaifeng Zhu [Fri, 7 Mar 2014 09:50:41 +0000 (09:50 +0000)]
readline: fix memory corruption when adding history

idx can be down to 0, so TERM_MAX_CMDS-idx+1 could be TERM_MAX_CMDS+1, which
exceeds the size of term_history.

Signed-off-by: Kaifeng Zhu <kaifeng.zhu@citrix.com>
Coverity-ID: 1055739
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agohw/msmouse.c: Fix deref_after_free and double free
Yunlei Ding [Mon, 17 Mar 2014 05:37:49 +0000 (05:37 +0000)]
hw/msmouse.c: Fix deref_after_free and double free

msmouse_chr_close is only pointed by chr->chr_close in qemu_chr_close
function. After calling chr->chr_close, chr will be freed. So we don't
need to free it again here.

Signed-off-by: Yunlei Ding <yunlei.ding@citrix.com>
(defect not identified by Coverity Scan)
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agosignal: Don't use uninitalised sival_ptr
Andrew Cooper [Tue, 21 Apr 2015 13:35:45 +0000 (14:35 +0100)]
signal: Don't use uninitalised sival_ptr

In 64bit builds, setting sival_int to 0 doesn't clear the upper half of the
sival_ptr pointer.  Valgrind does not like this.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agopic: Don't allocate irq buffers
Andrew Cooper [Tue, 21 Apr 2015 13:36:08 +0000 (14:36 +0100)]
pic: Don't allocate irq buffers

i8259_init() doesn't inspect its argument at all, causing the allocation to
be leaked and never used.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agosmbios: Don't allocate smbus eeprom buffer
Andrew Cooper [Tue, 21 Apr 2015 13:32:08 +0000 (14:32 +0100)]
smbios: Don't allocate smbus eeprom buffer

smbus_eeprom_device_init() has been disabled since 2007.  The #define turns
the actual function call into a comma expression with no effect.

Removing the leaked allocation also makes Valgrind happier.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Coverity-ID: 1090387

9 years agocmdline: Parse -pciemulation before trying to use it
Kaifeng Zhu [Thu, 8 Jan 2015 06:14:37 +0000 (06:14 +0000)]
cmdline: Parse -pciemulation before trying to use it

Signed-off-by: Kaifeng Zhu <kaifeng.zhu@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agodma: fix incorrect bh scheduling
Chunjie Zhu [Thu, 11 Dec 2014 12:18:02 +0000 (04:18 -0800)]
dma: fix incorrect bh scheduling

The following 2 cases should be avoided:

  1. DMAAIOCB has been released but continue_after_map_failure
     schedules a bh for it.
  2. Multiple bh calls are schduled on the same DMAAIOCB.

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agoide: cancel dma operations on command abort or error
Chunjie Zhu [Wed, 10 Dec 2014 10:36:23 +0000 (02:36 -0800)]
ide: cancel dma operations on command abort or error

Otherwise, a guest can cause Qemu to reuse an active aio structure.

Signed-off-by: Chunjie Zhu <chunjie.zhu@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agocirrus_vga: fix division by 0 for color expansion rop
Aurelien Jarno [Tue, 4 Jan 2011 20:58:24 +0000 (21:58 +0100)]
cirrus_vga: fix division by 0 for color expansion rop

Commit d85d0d3883f5a567fa2969a0396e42e0a662b3fa introduces a regression
with Windows ME that leads to a division by 0 and a crash.

It uses the color expansion rop with the source pitch set to 0. This is
something allowed, as the manual explicitely says "When the source of
color-expand data is display memory, the source pitch is ignored.".

This patch fixes this regression by computing sx, sy and others
variables only if they are going to be used later, that is for a plain
copy ROP. It basically consists in moving code.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
9 years agoCVE-2014-3615: vbe: rework sanity checks
Andrew Cooper [Sun, 22 Feb 2015 19:21:08 +0000 (19:21 +0000)]
CVE-2014-3615: vbe: rework sanity checks

Backport of qemu-upstream:
 * c1b886c45dc70f247300f549dce9833f3fa2def5

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agoCVE-2014-7815: vnc: sanitize bits_per_pixel from the client
Andrew Cooper [Sat, 21 Feb 2015 19:21:11 +0000 (19:21 +0000)]
CVE-2014-7815: vnc: sanitize bits_per_pixel from the client

Backport of qemu-upstream:
 * e6908bfe8e07f2b452e78e677da1b45b1c0f6829

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agoCVE-2014-8106: cirrus: fix blit region check
Andrew Cooper [Sat, 21 Feb 2015 17:16:42 +0000 (17:16 +0000)]
CVE-2014-8106: cirrus: fix blit region check

Backport of qemu-upstream:
 * bf25983345ca44aec3dd92c57142be45452bd38a
 * d3532a0db02296e687711b8cdc7791924efccea0

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agousb-linux.c: fix buffer overflow
Jim Paris [Wed, 22 Apr 2015 11:29:21 +0000 (12:29 +0100)]
usb-linux.c: fix buffer overflow

In usb-linux.c:usb_host_handle_control, we pass a 1024-byte buffer and
length to the kernel.  However, the length was provided by the caller
of dev->handle_packet, and is not checked, so the kernel might provide
too much data and overflow our buffer.

For example, hw/usb-uhci.c could set the length to 2047.
hw/usb-ohci.c looks like it might go up to 4096 or 8192.

This causes a qemu crash, as reported here:
  http://www.mail-archive.com/kvm@vger.kernel.org/msg18447.html

This patch increases the usb-linux.c buffer size to 2048 to fix the
specific device reported, and adds a check to avoid the overflow in
any case.

Signed-off-by: Jim Paris <jim@jtan.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
9 years agoblock-vvfat: fix fat_chksum() buffer overrun warning
Andrew Cooper [Tue, 4 Nov 2014 11:54:11 +0000 (11:54 +0000)]
block-vvfat: fix fat_chksum() buffer overrun warning

Newer GCC versions raise an undefined behaviour warning in
fat_chksum() because it overruns the name buffer.  However, this is
intentional behaviour because the extension array immediately follows.

Refactor this function to avoid the warning and make it clear it's
checksumming both parts.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Coverity-ID: 1055738

9 years agolm832x: don't overrun file buffer on save/restore
Andrew Cooper [Tue, 4 Nov 2014 11:46:46 +0000 (11:46 +0000)]
lm832x: don't overrun file buffer on save/restore

Saving and restoring an lm832x record would overrun the pwm.file array
since pwm.file is uint16_t elements and sizeof(pwm.file) twice as many
elements.

To ensure compatibility, padding bytes are added to the record.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Coverity-IDs: 1055728 1055729

9 years agocirrus_vga: default all I/O port reads to 0xff
Andrew Cooper [Tue, 4 Nov 2014 11:39:43 +0000 (11:39 +0000)]
cirrus_vga: default all I/O port reads to 0xff

Some error paths in vga_ioport_read() would return undefined values.
Always default the result to 0xff.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agovirtio-blk: initialise unused blkcfg.size_max field
Yunlei Ding [Fri, 18 Apr 2014 05:23:24 +0000 (06:23 +0100)]
virtio-blk: initialise unused blkcfg.size_max field

Newer GCC warns about memcpy()ing uninitialised data.

Signed-off-by: Yunlei Ding <yunlei.ding@citrix.com>
Coverity-ID: 1056088
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
9 years agoFix build after "ui/vnc: limit client_cut_text msg payload size" xen-4.6.0 xen-4.6.0-rc4 xen-4.6.0-rc5
Ian Jackson [Tue, 8 Sep 2015 14:41:20 +0000 (15:41 +0100)]
Fix build after "ui/vnc: limit client_cut_text msg payload size"

That backport (which also erroneously refers to ui/vnc.c rather than
vnc.c in its commit message) broke the build because we do not have
`error_report'.  Use fprintf to stderr, like the rest of the file.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
9 years agoui/vnc: limit client_cut_text msg payload size
Peter Lieven [Mon, 30 Jun 2014 08:07:54 +0000 (10:07 +0200)]
ui/vnc: limit client_cut_text msg payload size

currently a malicious client could define a payload
size of 2^32 - 1 bytes and send up to that size of
data to the vnc server. The server would allocated
that amount of memory which could easily create an
out of memory condition.

This patch limits the payload size to 1MB max.

Please note that client_cut_text messages are currently
silently ignored.

Signed-off-by: Peter Lieven <pl@kamp.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
(cherry picked from commit f9a70e79391f6d7c2a912d785239ee8effc1922d)
Conflicts:
ui/vnc.c

Dropped { } style changes.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
9 years agoHVM: atomically access pointers in bufioreq handling
Ian Jackson [Fri, 28 Aug 2015 14:53:40 +0000 (15:53 +0100)]
HVM: atomically access pointers in bufioreq handling

The number of slots per page being 511 (i.e. not a power of two) means
that the (32-bit) read and write indexes going beyond 2^32 will likely
disturb operation. The hypervisor side gets I/O req server creation
extended so we can indicate that we're using suitable atomic accesses
where needed, allowing it to atomically canonicalize both pointers when
both have gone through at least one cycle.

The Xen side counterpart (which is not a functional prereq to this
change, albeit the intention is for Xen to assume default servers
always use suitable atomic accesses) went in already (commit
b7007bc6f9).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v2: Adjust description.

9 years agoide: Clear DRQ after handling all expected accesses xen-4.6.0-rc1 xen-4.6.0-rc2
Kevin Wolf [Wed, 3 Jun 2015 12:41:27 +0000 (14:41 +0200)]
ide: Clear DRQ after handling all expected accesses

This is additional hardening against an end_transfer_func that fails to
clear the DRQ status bit. The bit must be unset as soon as the PIO
transfer has completed, so it's better to do this in a central place
instead of duplicating the code in all commands (and forgetting it in
some).

[ This is XSA-138 / CVE-2015-5154. ]

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
9 years agoide: Check array bounds before writing to io_buffer (CVE-2015-5154)
Kevin Wolf [Wed, 3 Jun 2015 12:13:31 +0000 (14:13 +0200)]
ide: Check array bounds before writing to io_buffer (CVE-2015-5154)

If the end_transfer_func of a command is called because enough data has
been read or written for the current PIO transfer, and it fails to
correctly call the command completion functions, the DRQ bit in the
status register and s->end_transfer_func may remain set. This allows the
guest to access further bytes in s->io_buffer beyond s->data_end, and
eventually overflowing the io_buffer.

One case where this currently happens is emulation of the ATAPI command
START STOP UNIT.

This patch fixes the problem by adding explicit array bounds checks
before accessing the buffer instead of relying on end_transfer_func to
function correctly.

[ This is XSA-138 / CVE-2015-5154. ]

Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
9 years agopcnet: force the buffer access to be in bounds during tx
Petr Matousek [Sun, 24 May 2015 08:53:44 +0000 (10:53 +0200)]
pcnet: force the buffer access to be in bounds during tx

4096 is the maximum length per TMD and it is also currently the size of
the relay buffer pcnet driver uses for sending the packet data to QEMU
for further processing. With packet spanning multiple TMDs it can
happen that the overall packet size will be bigger than sizeof(buffer),
which results in memory corruption.

Fix this by only allowing to queue maximum sizeof(buffer) bytes.

This is CVE-2015-3209.

Signed-off-by: Petr Matousek <pmatouse@redhat.com>
Reported-by: Matt Tait <matttait@google.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
9 years agopcnet: fix Negative array index read
Gonglei [Tue, 23 Jun 2015 10:42:13 +0000 (11:42 +0100)]
pcnet: fix Negative array index read

s->xmit_pos maybe assigned to a negative value (-1),
but in this branch variable s->xmit_pos as an index to
array s->buffer. Let's add a check for s->xmit_pos.

upstream-commit-id: 7b50d00911ddd6d56a766ac5671e47304c20a21b

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
9 years ago... by default. Add a per-device "permissive" mode similar to pciback's
Jan Beulich [Tue, 9 Jun 2015 15:13:11 +0000 (16:13 +0100)]
... by default. Add a per-device "permissive" mode similar to pciback's
to allow restoring previous behavior (and hence break security again,
i.e. should be used only for trusted guests).

This is part of XSA-131.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>)
9 years agoSince the next patch will turn all not explicitly described fields
Jan Beulich [Tue, 9 Jun 2015 15:13:11 +0000 (16:13 +0100)]
Since the next patch will turn all not explicitly described fields
read-only by default, those fields that have guest writable bits need
to be given explicit descriptors.

This is a preparatory patch for XSA-131.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
9 years agoThe adjustments are solely to make the subsequent patches work right
Jan Beulich [Tue, 9 Jun 2015 15:13:11 +0000 (16:13 +0100)]
The adjustments are solely to make the subsequent patches work right
(and hence make the patch set consistent), namely if permissive mode
(introduced by the last patch) gets used (as both reserved registers
and reserved fields must be similarly protected from guest access in
default mode, but the guest should be allowed access to them in
permissive mode).

This is a preparatory patch for XSA-131.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
9 years agoxen_pt_emu_reg_pcie[]'s PCI_EXP_DEVCAP needs to cover all bits as read-
Jan Beulich [Tue, 9 Jun 2015 15:13:11 +0000 (16:13 +0100)]
xen_pt_emu_reg_pcie[]'s PCI_EXP_DEVCAP needs to cover all bits as read-
only to avoid unintended write-back (just a precaution, the field ought
to be read-only in hardware).

This is a preparatory patch for XSA-131.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
9 years agoThis is just to avoid having to adjust that calculation later in
Jan Beulich [Tue, 9 Jun 2015 15:13:11 +0000 (16:13 +0100)]
This is just to avoid having to adjust that calculation later in
multiple places.

Note that including ->ro_mask in get_throughable_mask()'s calculation
is only an apparent (i.e. benign) behavioral change: For r/o fields it
doesn't matter > whether they get passed through - either the same flag
is also set in emu_mask (then there's no change at all) or the field is
r/o in hardware (and hence a write won't change it anyway).

This is a preparatory patch for XSA-131.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
9 years agoxen_pt_pmcsr_reg_write() needs an adjustment to deal with the RW1C
Jan Beulich [Tue, 9 Jun 2015 15:13:11 +0000 (16:13 +0100)]
xen_pt_pmcsr_reg_write() needs an adjustment to deal with the RW1C
nature of the not passed through bit 15 (PCI_PM_CTRL_PME_STATUS).

This is a preparatory patch for XSA-131.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
9 years agoThere's no point in xen_pt_pmcsr_reg_{read,write}() each ORing
Jan Beulich [Tue, 9 Jun 2015 15:13:11 +0000 (16:13 +0100)]
There's no point in xen_pt_pmcsr_reg_{read,write}() each ORing
PCI_PM_CTRL_STATE_MASK and PCI_PM_CTRL_NO_SOFT_RESET into a local
emu_mask variable - we can have the same effect by setting the field
descriptor's emu_mask member suitably right away. Note that
xen_pt_pmcsr_reg_write() is being retained in order to allow later
patches to be less intrusive.

This is a preparatory patch for XSA-131.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
9 years agoWithout this the actual XSA-131 fix would cause the enable bit to not
Jan Beulich [Tue, 9 Jun 2015 15:13:11 +0000 (16:13 +0100)]
Without this the actual XSA-131 fix would cause the enable bit to not
get set anymore (due to the write back getting suppressed there based
on the OR of emu_mask, ro_mask, and res_mask).

Note that the fiddling with the enable bit shouldn't really be done by
qemu, but making this work right (via libxc and the hypervisor) will
require more extensive changes, which can be postponed until after the
security issue got addressed.

This is a preparatory patch for XSA-131.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
9 years ago... to avoid allowing the guest to cause the control domain's disk to
Jan Beulich [Tue, 9 Jun 2015 15:13:00 +0000 (16:13 +0100)]
... to avoid allowing the guest to cause the control domain's disk to
fill.

This is XSA-130.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
9 years agoIt's being used by the hypervisor. For now simply mimic a device not
Jan Beulich [Tue, 9 Jun 2015 15:12:55 +0000 (16:12 +0100)]
It's being used by the hypervisor. For now simply mimic a device not
capable of masking, and fully emulate any accesses a guest may issue
nevertheless as simple reads/writes without side effects.

This is XSA-129.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
9 years agoThe old logic didn't work as intended when an access spanned multiple
Jan Beulich [Tue, 9 Jun 2015 15:12:30 +0000 (16:12 +0100)]
The old logic didn't work as intended when an access spanned multiple
fields (for example a 32-bit access to the location of the MSI Message
Data field with the high 16 bits not being covered by any known field).
Remove it and derive which fields not to write to from the accessed
fields' emulation masks: When they're all ones, there's no point in
doing any host write.

This fixes a secondary issue at once: We obviously shouldn't make any
host write attempt when already the host read failed.

This is XSA-128.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
9 years agofdc: force the fifo access to be in bounds of the allocated buffer
Petr Matousek [Wed, 6 May 2015 07:48:59 +0000 (09:48 +0200)]
fdc: force the fifo access to be in bounds of the allocated buffer

During processing of certain commands such as FD_CMD_READ_ID and
FD_CMD_DRIVE_SPECIFICATION_COMMAND the fifo memory access could
get out of bounds leading to memory corruption with values coming
from the guest.

Fix this by making sure that the index is always bounded by the
allocated memory.

This is XSA-133 / CVE-2015-3456.

Signed-off-by: Petr Matousek <pmatouse@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
10 years agoqemu-trad: xenstore: use relative path for device-model node
Wei Liu [Thu, 9 Apr 2015 18:49:24 +0000 (19:49 +0100)]
qemu-trad: xenstore: use relative path for device-model node

For QEMU traditional running in Dom0, there is no functional change
because it will still write to the same /local/domain/0 path.

For QEMU traditional stubdom, this is an incompatible startup
protocol change.  There is a corresponding libxl changeset "libxl:
use new QEMU xenstore protocol", which has not yet been committed
(54c2e621 was reverted in 84066dd4).

QEMU traditional stubdom was broken by a0731cca "ioreq-server: on-demand
creation of ioreq server" in 4.5. Currently there is a workaround in
-unstable dd748d12 "x86/hvm: wait for at least one ioreq server to be
enabled" (which should be backported to 4.5). QEMU traditional stubdom
works with that workaround in -unstable but it's not ideal situation.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
10 years agoxen: limit guest control of PCI command register
Jan Beulich [Tue, 31 Mar 2015 15:27:45 +0000 (16:27 +0100)]
xen: limit guest control of PCI command register

Otherwise the guest can abuse that control to cause e.g. PCIe
Unsupported Request responses (by disabling memory and/or I/O decoding
and subsequently causing [CPU side] accesses to the respective address
ranges), which (depending on system configuration) may be fatal to the
host.

This is CVE-2015-2756 / XSA-126.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
10 years agocirrus: fix an uninitialized variable
Jan Beulich [Mon, 2 Feb 2015 16:49:59 +0000 (16:49 +0000)]
cirrus: fix an uninitialized variable

Noticed by paying attention to a compiler warning (reported for the
only caller of the function modified, vga_ioport_read()).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
10 years agoqemu-xen-trad: Switch to $(LIBEXEC_BIN) from $(LIBEXEC) xen-4.5.0 xen-4.5.0-rc1
Ian Campbell [Mon, 6 Oct 2014 15:24:46 +0000 (16:24 +0100)]
qemu-xen-trad: Switch to $(LIBEXEC_BIN) from $(LIBEXEC)

The meaning of LIBEXEC in the Xen build system has been made to
follow the more conventional meaning.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
10 years agoqemu-xen-trad: -I tools/libxc/include and tools/xenstore/include
Stefano Stabellini [Fri, 1 Aug 2014 15:32:18 +0000 (16:32 +0100)]
qemu-xen-trad: -I tools/libxc/include and tools/xenstore/include

The public libxc and xenstore headers have been moved to
tools/libxc/include and tools/xenstore/include respectively: change the
Makefiles accordingly.

10 years agoqemu-xen-trad: free all the pirqs for msi/msix when driver unloads
Zhenzhong Duan [Tue, 8 Apr 2014 16:50:06 +0000 (16:50 +0000)]
qemu-xen-trad: free all the pirqs for msi/msix when driver unloads

Pirqs are not freed when driver unloads, then new pirqs are allocated when
driver reloads. This could exhaust pirqs if do it in a loop.

This patch fixes the bug by freeing pirqs when ENABLE bit is cleared in
msi/msix control reg.

There is also other way of fixing it such as reuse pirqs between driver reload,
but this way is better.
Xen-devel: http://marc.info/?l=xen-devel&m=136800120304275&w=2

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
[ This is the qemu-xen-trad version of qemu-xen-upstream
  1d4fd4f0e2fc5dcae0c60e00cc9af95f52988050 -iwj ]

11 years agoqemu-traditional: Fix build warnings on Wheezy xen-4.4.0 xen-4.4.0-rc2 xen-4.4.0-rc3 xen-4.4.0-rc4 xen-4.4.0-rc5 xen-4.4.0-rc6
Andrew Cooper [Wed, 18 Dec 2013 15:25:14 +0000 (15:25 +0000)]
qemu-traditional: Fix build warnings on Wheezy

  CC    i386-dm/eepro100.o
hw/eepro100.c: In function ‘eepro100_read4’:
hw/eepro100.c:1232:5: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
hw/eepro100.c: In function ‘eepro100_read2’:
hw/eepro100.c:1202:5: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]
hw/eepro100.c: In function ‘eepro100_read1’:
hw/eepro100.c:1179:5: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized]

Use ~0 to match the behaviour of real hardware.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
11 years agoxen_disk: fix memory leak xen-4.4.0-rc1
Matthew Daley [Wed, 4 Dec 2013 02:16:18 +0000 (15:16 +1300)]
xen_disk: fix memory leak

On ioreq_release the full ioreq was memset to 0, losing all the data
and memory allocations inside the QEMUIOVector, which leads to a
memory leak. Create a new function to specifically reset ioreq.

Reported-by: Maik Wessler <maik.wessler@yahoo.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Backport to qemu-xen-traditional.

Signed-off-by: Matthew Daley <mattd@bugfuzz.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
11 years agohw/piix4acpi: Make writes to ACPI_DBG_IO_ADDR actually work.
Konrad Rzeszutek Wilk [Mon, 11 Nov 2013 18:42:56 +0000 (13:42 -0500)]
hw/piix4acpi: Make writes to ACPI_DBG_IO_ADDR actually work.

The ACPI AML code has little snippets where it uses two
memory locations to stash debug information when doing PCI
hotplug, such as:

Device (S20)
{
    Name (_ADR, 0x00040000)
    Name (_SUN, 0x04)
    Method (_EJ0, 1, NotSerialized)
    {
Store (0x20, \_GPE.DPT1)
Store (0x88, \_GPE.DPT2)
Store (One, \_GPE.PH20)
    }

    Method (_STA, 0, NotSerialized)
    {
Store (0x20, \_GPE.DPT1)
Store (0x89, \_GPE.DPT2)
    }
}

and DPT1 (and DPT2) is defined as:

OperationRegion ( DG1, SystemIO, 0xb044, 0x04 )
Field ( DG1, ByteAcc, NoLock, Preserve ) {
    DPT1, 8, DPT2, 8
}

But unfortunately when we do the writes they are done
as byte writes, not as 4-byte writes (long). Hence
any debug values are never show in QEMU.

This fixes it so that we can see them in the log file.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
11 years agoqemu-traditional: do not strip binaries during make install
Olaf Hering [Tue, 15 Oct 2013 09:42:26 +0000 (11:42 +0200)]
qemu-traditional: do not strip binaries during make install

It is wrong to strip code during make install, unless explicit
requested. Introduce a new variable INSTALL_PROG and use it along with
an optional STRIP_OPT where currently install -s -m 755 is used.
This is what upstream qemu offers in version 1.6.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
11 years agoqemu-xen-traditional: allow build without blktap1
Ian Campbell [Wed, 31 Jul 2013 15:16:16 +0000 (16:16 +0100)]
qemu-xen-traditional: allow build without blktap1

I intend this to become optional at the xen build level so it needs to become
optional here. Until the matching Xen patch is applied and exports
CONFIG_BLKTAP1=y|n there should be no change.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
11 years agopassthrough: Correctly expose PCH ISA bridge for IGD passthrough
G.R [Thu, 25 Jul 2013 08:11:55 +0000 (08:11 +0000)]
passthrough: Correctly expose PCH ISA bridge for IGD passthrough

The i915 driver probes chip version through PCH ISA bridge device / vendor ID.
Previously, the PCH ISA bridge is exposed as PCI-PCI bridge in qemu-xen-trad,
which breaks the assumption of the driver. This change fixes the issue by
correctly exposing the ISA bridge to domU.

Signed-off-by: Rui Guo <firemeteor@users.sourceforge.net>
Tested-by: Rui Guo <firemeteor@users.sourceforge.net>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Jan Beulich <JBeulich@suse.com>
11 years agoqemu-xen-traditional: disable docs xen-4.3.0
Ian Jackson [Mon, 17 Jun 2013 16:39:51 +0000 (17:39 +0100)]
qemu-xen-traditional: disable docs

Perl 5.18 is unhappy with the qemu-xen-traditional documents, giving
this error:
  qemu.pod around line 91: Non-ASCII character seen before =encoding in
  'Schuetz.'. Assuming UTF-8 POD document had syntax errors at /usr/bin/core_pe\
rl/pod2man line 71.
  make[3]: *** [qemu.1] Error 255

We do not want these docs.  They are not really relevant to Xen users.
So instead of backporting the utf-8 fix from qemu upstream
(3179d694a8dcaa091131e3db644d445c0130713e), we just disable the docs
build.

We do this in xen-hooks.mak because qemu-xen-traditional's configure
script lacks the ability to explicitly disable the docs build.

(The docs build for upstream-based qemu-xen was already disabled by
xen.git#a0d110801d701c43e7b8c73dbd6b2444a10a7cdb)

Reported-by: jacek burghardt <jaceksburghardt@gmail.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com> (re 4.3 release)
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
11 years agoSigned-off-by: Olaf Hering <olaf@aepfle.de>
Ian Jackson [Tue, 21 May 2013 13:57:23 +0000 (14:57 +0100)]
Signed-off-by: Olaf Hering <olaf@aepfle.de>
CC: Eric Shelton <eshelton@pobox.com>
CC: Matt Wilson <msw@amazon.com>
CC: Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>
Acked-by: George Dunlap <george.dunlap@eu> (for 4.3 release)
11 years agopiix4acpi, xen, hotplug: Fix race with ACPI AML code and hotplug.
Konrad Rzeszutek Wilk [Tue, 14 May 2013 17:48:49 +0000 (18:48 +0100)]
piix4acpi, xen, hotplug: Fix race with ACPI AML code and hotplug.

This is a race so the amount varies but on a 4PCPU box
I seem to get only ~14 out of 16 vCPUs I want to online.

The issue at hand is that QEMU xenstore.c hotplug code changes
the vCPU array and triggers an ACPI SCI for each vCPU
online/offline change. That means we modify the array of vCPUs
as the guests ACPI AML code is reading it - resulting in
the guest reading the data only once and not changing the
CPU states appropiately.

The fix is to seperate the vCPU array changes from the ACPI SCI
notification. The code now will enumerate all of the vCPUs
and change the vCPU array if there is a need for a change.
If a change did occur then only _one_ ACPI SCI pulse is sent
to the guest. The vCPU array at that point has the online/offline
modified to what the user wanted to have.

Specifically, if a user provided this command:
 xl vcpu-set latest 16

(guest config has vcpus=1, maxvcpus=32) QEMU and the guest
(in this case Linux) would do:

QEMU:                                           Guest OS:
-xenstore_process_vcpu_set_event
 -> Gets an XenBus notification for CPU1
 -> Updates the gpe_state.cpus_state bitfield.
        -> Pulses the ACPI SCI
                                                - ACPI SCI kicks in

 -> Gets an XenBus notification for CPU2
 -> Updates the gpe_state.cpus_state bitfield.
        -> Pulses the ACPI SCI

 -> Gets an XenBus notification for CPU3
 -> Updates the gpe_state.cpus_state bitfield.
        -> Pulses the ACPI SCI
   ...
                                                 - Method(PRST) invoked

 -> Gets an XenBus notification for CPU12
 -> Updates the gpe_state.cpus_state bitfield.
        -> Pulses the ACPI SCI
                                                  - reads AF00 for CPU state
                                                    [gets 0xff]
                                                  - reads AF02 [gets 0x7f]

 -> Gets an XenBus notification for CPU13
 -> Updates the gpe_state.cpus_state bitfield.
        -> Pulses the ACPI SCI

        .. until VCPU 16
                                                 - Method PRST updates
                                                   PR01 through 13 FLG
                                                   entry.
                                                 - PR01->PR13 _MAD
                                                   invoked.

                                                 - Brings up 13 CPUs.

While QEMU updates the rest of the cpus_state bitfields the ACPI AML
only does the CPU hotplug on those it had read.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com> (for 4.3 release)
11 years agopiix4acpi, xen: Clarify that the qemu_set_irq calls just do an IRQ pulse.
Konrad Rzeszutek Wilk [Tue, 14 May 2013 17:48:48 +0000 (18:48 +0100)]
piix4acpi, xen: Clarify that the qemu_set_irq calls just do an IRQ pulse.

The "qemu_cpu_notify" raises and lowers the ACPI SCI line when the
vCPU state has changed.

Instead of doing the two functions, just use one function that
describes exactly what it does.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com> (for 4.3 release)
11 years agopiix4acpi, xen, vcpu hotplug: Split the notification from the changes.
Konrad Rzeszutek Wilk [Tue, 14 May 2013 17:48:47 +0000 (18:48 +0100)]
piix4acpi, xen, vcpu hotplug: Split the notification from the changes.

This is a prepatory patch that splits the notification
of an vCPU change from the actual changes to the vCPU array.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com> (for 4.3 release)
12 years agoxen: Disable use of O_DIRECT by default as it results in crashes. xen-4.3.0-rc1
Alex Bligh [Fri, 15 Mar 2013 18:25:15 +0000 (18:25 +0000)]
xen: Disable use of O_DIRECT by default as it results in crashes.

Due to what is almost certainly a kernel bug, writes with O_DIRECT may
continue to reference the page after the write has been marked as
completed, particularly in the case of TCP retransmit. In other
scenarios, this "merely" risks data corruption on the write, but with
Xen pages from domU are only transiently mapped into dom0's memory,
resulting in kernel panics when they are subsequently accessed.

This brings PV devices in line with emulated devices.  Removing
O_DIRECT is safe as barrier operations are now correctly passed
through.

See:
  http://lists.xen.org/archives/html/xen-devel/2012-12/msg01154.html
for more details.

Signed-off-by: Alex Bligh <alex@alex.org.uk>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Backported to qemu-xen-unstable,
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
12 years agoRevert "passthrough: Correctly expose PCH ISA bridge for IGD passthrough"
Ian Jackson [Mon, 25 Feb 2013 16:44:20 +0000 (16:44 +0000)]
Revert "passthrough: Correctly expose PCH ISA bridge for IGD passthrough"

This reverts commit bd9e97271db5edc07e3e0d45bdf6ccd5a9bba3de.

Revert-requested-by: Jan Beulich <JBeulich@suse.com>
12 years agopassthrough: Correctly expose PCH ISA bridge for IGD passthrough
Ian Jackson [Fri, 22 Feb 2013 18:04:40 +0000 (18:04 +0000)]
passthrough: Correctly expose PCH ISA bridge for IGD passthrough

Fix IGD passthrough logic to properly expose PCH ISA bridge (instead
of exposing as pci-pci bridge). The i915 driver require this to
correctly detect the PCH version and enable version specific code
path.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Timothy Guo <firemeteor@users.sourceforge.net>