Commit Graph
10613 Commits
Author SHA1 Message Date
Ian Jackson b1b7d5293a libxl: Cleanup: Have libxl__alloc_vdev use /libxl
When allocating a vdev for a new disk, look in /libxl/device, rather
than the frontends directory in xenstore.

This is more in line with the other parts of libxl, which ought not to
trust frontends.  In this case, though, there is no security bug prior
to this patch because the frontend is the toolstack domain itself.

If libxl__alloc_vdev were ever changed to take a frontend domain
argument, this patch will fix a latent security bug.

This is a followup to XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:22:23 +01:00
Ian Jackson 08ea21f236 libxl: Do not trust backend for nic in list
libxl_device_nic_list should use the /libxl path to search for
devices, and for obtaining the device information.

The "type" parameter was always "vif".  Abolish it.  (In any case,
paths in /libxl/device are named after the frontend type which is
constant, not the backend type which might in future vary.)

Abolish a redundant store to pnic->backend_domid.  Before this commit,
that store was not needed because libxl_device_nic_init (called by
libxl__device_nic_from_xenstore) would zero it.  Now it overwrites the
correct backend domid with zero; so remove it.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:22:19 +01:00
Ian Jackson 30f1276f1d libxl: Do not trust backend for nic in devid_to_device
libxl_devid_to_device_nic should read the information it needs from
the /libxl/device path, not the backend.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:22:17 +01:00
Ian Jackson 492494f47b libxl: Do not trust backend in nic getinfo
This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:21:57 +01:00
Ian Jackson 399dabb4e0 libxl: Have READ_LIBXLDEV use libxl_path rather than be_path
Fix the just-introduced bug in this macro: now it reads the
trustworthy libxl_path.  Change the variable name in the two functions
(nic and channel) which use it.

Shuffling the bump in the carpet along, we now introduce three new
bugs: the three call sites pass a backend path where a frontend path
is expected.

No functional change.

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:21:52 +01:00
Ian Jackson 06fa494bb5 libxl: Rename libxl__device_nic_from_xs_be to _from_xenstore
We are going to change these functions to expect, and be passed, a
/libxl path.  So it is wrong that they are called _from_xs_be.

Neither function reads anything which isn't found in both places, so
we can and will change the call sites later.

The only remaining function in libxl called *_from_xs_be relates to
PCI devices, for which the backend domain is hardcoded to 0 throughout
the libxl_pci.c.

No functional change.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:21:47 +01:00
Ian Jackson 57333d5561 libxl: Do not trust backend for cdrom insert
Use the /libxl path where appropriate.  Rename `path' variable to
`be_path' to make sure we caught all the occurrences.

Specifically, when checking that the device still exists, check the
`frontend' value in /libxl, rather than anything in the backend
directory.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:21:43 +01:00
Ian Jackson 1e213d5068 libxl: Do not trust backend for disk in getinfo
Do not read the frontend path out of the backend.  We have it in our
hand.  Likewise the guest (frontend) domid was one of our parameters (!)

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:21:40 +01:00
Ian Jackson 04e7c06b92 libxl: Do not trust backend for disk; fix driver domain disks list
Rework libxl__device_disk_from_xs_be (which takes a backend path) into
to libxl__device_disk_from_xenstore (which takes a libxl path).

libxl__device_disk_from_xenstore now finds the backend path itself,
although it doesn't use it any more for most of its functions.  We
rename the variable from be_path to backend_path to make sure we
didn't miss any cases.

All the data collection is now done by reading from the copy in
/libxl.

libxl_device_disk_list and its helper libxl__append_disk_list (which
used to be libxl__append_disk_list_of_type) need extensive rework,
because they now need to specify the /libxl path rather than the
backend path.

To do that they enumerate disks by looking in the appropriate area in
/libxl.  Previously they scanned various of the backend directories in
dom0 (which was broken for driver domains).  It is no longer necessary
to enumerate the various disk backends, because they all use the same
paths in /devices.  libxl__device_disk_from_xenstore will parse the
type out of the backend path, for itself.  (Indeed, it did so before -
the now-gone type parameter to libxl__append_disk_list_of_type wasn't
used other than to construct the directory to list.)

Finally, remove a redundant store to pdisk->backend_domid in
libxl__append_disk_list[_of_type].  Even before this commit, that
store was not needed because libxl_device_disk_init (called by
libxl__device_disk_from_xenstore) would zero it.  Now it overwrites
the correct backend domid with zero; so remove it.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
v2: Also fix up COLO reads, following rebase
2016-06-06 17:21:38 +01:00
Ian Jackson 728fcedbc3 libxl: Do not trust backend for disk eject vdev
For disk eject, use configured vdev from /libxl, not backend.

The backend directory is writeable by driver domains.  This means that
a malicious driver domain could cause libxl to see a wrong vdev,
confusing the user or the toolstack.

Use the vdev from the /libxl space, rather than the backend.

For convenience, we read the vdev from the /libxl space into the evg
during setup and copy it on each event, rather than reading it afresh
each time (which would in any case involve generating or saving a copy
of the relevant /libxl path).

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:12:10 +01:00
Ian Jackson 17916bae12 libxl: cdrom eject and insert: write to /libxl
Copy the new type and params values to /libxl, so that the information
in /libxl is kept up to date.

This is needed so that we can return this trustworthy information,
rather than trusting the backend-writeable parts of xenstore.

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:11:28 +01:00
Ian Jackson 424d83ea2a libxl: Do not trust backend for vtpm in getinfo (uuid)
Use uuid from /libxl, rather than from backend.  I think the backend
is not supposed to change the uuid, since it seems to be set by libxl
during setup.

If in fact the backend is supposed to be able to change the uuid, this
patch needs to be dropped and replaced by a patch which makes the vtpm
uuid lookup tolerate bad or missing data.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:11:28 +01:00
Ian Jackson d920eaa482 libxl: Do not trust backend for vtpm in getinfo (except uuid)
* Do not check the backend for existence.  We have already read the
  /libxl path so know that the vtpm exists (or is supposed to); if the
  backend doesn't exist then that must be the backend's doing.
* Get the frontend path from the /libxl directory.
* The frontend domid is the guest domid, and does not need to be read
  from xenstore (!)

We still attempt to read the uuid from the backend.  This will be
fixed in the next patch.

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:11:28 +01:00
Ian Jackson 9f72a8647b libxl: Make copy of every xs backend in /libxl in _generic_add
We want to stop libxl trustingly reading information from the backend
directory (since this is, of course, writeable by the backend, which
might be a semi-trusted driver domain).

In principle it is wrong in current libxl for anything to try to
divine virtual device configuration from xenstore: the JSON domain
config ought to supply that, and xenstore should only tell us which
devices actually exist.

However:

Firstly, there are several existing places where configuration
information is retrieved from xenstore rather than JSON.  We do not
want to reen gineer this in a security patch.

Secondly, we want to make a security patch which can be backported to
versions of libxl without the JSON configuration machinery.

So we take the expedient approach of keeping a copy of the
configuration somewhere we trust, namely /libxl.  This is obviously
fairly low-risk, although it does write significantly more keys in
xenstore.

In this patch we make this change in libxl__device_generic_add.  This
is responsible for actually writing the vast majority of device
information to xenstore.  There are a few loose ends which will be
dealt with in a moment.

Likewise, changes to readers to use the new location will appear in
further patches.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:11:28 +01:00
Ian Jackson 5811d6bdf5 libxl: Do not trust frontend for nic in getinfo
libxl_device_nic_getinfo needs to examine devices without trusting
frontend-controlled data.  So:

* Use /libxl to find the backend path.
* Parse the backend path to find the backend domid, rather than
  reading it from the frontend.

This is part of XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:06:44 +01:00
Ian Jackson 2c74b8dd4c libxl: Do not trust frontend for nic in libxl_devid_to_device_nic
Find the backend by reading the pointer in /libxl rather than in the
guest's frontend area.

This is part of XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:06:44 +01:00
Ian Jackson 3544831133 libxl: Do not trust frontend for vtpm in getinfo
libxl_device_vtpm_getinfo needs to examine devices without trusting
frontend-controlled data.  So:

* Use /libxl to find the backend path.
* Parse the backend path to find the backend domid, rather than
  reading it from the frontend.

This is part of XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:06:44 +01:00
Ian Jackson 7bde3f4c5d libxl: Do not trust frontend for vtpm list
libxl_device_vtpm_list needs to enumerate and identify devices without
trusting frontend-controlled data.  So

* Use the /libxl path to enumerate vtpms.
* Use the /libxl path to find the corresponding backends.
* Parse the backend path to find the backend domid.

This is part of XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:06:44 +01:00
Ian Jackson 38ef6689b7 libxl: Do not trust frontend for disk in getinfo
* Rename the frontend variable to `fe_path' to check we caught them all
* Read the backend path from /libxl, rather than from the frontend
* Parse the backend domid from the backend path, rather than reading it
  from the frontend (and add the appropriate error path and initialisation)

This is part of XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:06:43 +01:00
Ian Jackson babf4f45a5 libxl: Do not trust frontend for disk eject event
Use the /libxl path for interpreting disk eject watch events: do not
read the backend path out of the frontend.  Instead, use the version
in /libxl.  That avoids us relying on the guest-modifiable
$frontend/backend pointer.

To implement this we store the path
  /libxl/$guest/device/vbd/$devid/backend
in the evgen structure.

This is part of XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:06:43 +01:00
Ian Jackson 13de0ee031 libxl: Do not trust frontend in libxl__device_nextid
When selecting the devid for a new device, we should look in
/libxl/device for existing devices, not in the frontend area.

This is part of XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:06:43 +01:00
Ian Jackson 4a78d36024 libxl: Do not trust frontend in libxl__devices_destroy
We need to enumerate the devices we have provided to a domain, without
trusting the guest-writeable (or, at least, guest-deletable) frontend
paths.

Instead, enumerate via, and read the backend path from, /libxl.

The console /libxl path is regular, so the special case for console 0
is not relevant any more: /libxl/GUEST/device/console/0 will be found,
and then libxl__device_destroy will DTRT to the right frontend path.

This is part of XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:06:43 +01:00
Ian Jackson 89c6672d01 libxl: Provide libxl__backendpath_parse_domid
Multiple places in libxl need to figure out the backend domid of a
device.  This can be discovered easily by looking at the backend path,
which always starts /local/domain/$backend_domid/.

There are no call sites yet.

This is part of XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:05:30 +01:00
Ian Jackson 44ea68c6e6 libxl: Record backend/frontend paths in /libxl/$DOMID
This gives us a record of all the backends we have set up for a
domain, which is separate from the frontends in
  /local/domain/$DOMID/device.

In particular:

1. A guest has write permission for the frontend path:
  /local/domain/$DOMID/device/$KIND/$DEVID
which means that the guest can completely delete the frontend.
(They can't recreate it because they don't have write permission
on the containing directory.)

2. A guest has write permission for the backend path recorded in the
frontend, ie, it can write to
  /local/domain/$DOMID/device/$KIND/$DEVID/backend
which means that the guest can break the association between
frontend and backend.

So we can't rely on iterating over the frontends to find all the
backends, or examining a frontend to discover how a device is
configured.

So, have libxl__device_generic_add record the frontend and backend
paths in /libxl/$DOMID/device, and have libxl__device_destroy remove
them again.

Create the containing directory /libxl/GUEST/device in
libxl__domain_make.  The already existing xs_rm in devices_destroy_cb
will take care of removing it.

This is part of XSA-175.

Backport note: Backported over 7472ced, which fixes a bug in driver
domain teardown.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 17:05:30 +01:00
Konrad Rzeszutek Wilk 4db81940ee libvchan: Read prod/cons only once.
We must ensure that the prod/cons are only read once and that
the compiler won't try to optimize the reads. That is split
the read of these in multiple instructions influencing later
branch code. As such insert barriers when fetching the cons
and prod index.

This is part of XSA155.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
master commit: 7d66a4ba69
master date: 2015-12-18 09:50:02 -0500
2016-03-10 08:27:21 +01:00
Ian Campbell 404e83e055 tools: pygrub: if partition table is empty, try treating as a whole disk
pygrub (in identify_disk_image()) detects a DOS style partition table
via the presence of the 0xaa55 signature at the end of the first
sector of the disk.

However this signature is also present in whole-disk configurations
when there is an MBR on the disk. Many filesystems (e.g. ext[234])
include leading padding in their on disk format specifically to enable
this.

So if we think we have a DOS partition table but do not find any
actual partition table entries we may as well try looking at it as a
whole disk image. Worst case is we probe and find there isn't anything
there.

This was reported by Sjors Gielen in Debian bug #745419. The fix was
inspired by a patch by Adi Kriegisch in
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=745419#27

Tested by genext2fs'ing my /boot into a new raw image (works) and
then:
   dd if=/usr/lib/grub/i386-pc/g2ldr.mbr of=img conv=notrunc bs=512 count=1

to add an MBR (with 0xaa55 signature) to it, which after this patch
also works.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: 745419-forwarded@bugs.debian.org
(cherry picked from commit fb31b1475f)
(cherry picked from commit 6c9b1bcce4)
(cherry picked from commit 812406cf2b)
(cherry picked from commit 83c5e46c5d)
2016-02-29 15:48:14 +00:00
Dario Faggioli a64bfae49a blktap: Fix two 'maybe uninitialized' variables
[ Cross-ported to blktap1 from 345e44a85d
  "blktap2: Fix two 'maybe uninitialized' variables" -iwj;
  Remainder of commit message is from blktap2's version. ]

for which gcc 4.9.0 complains about, like this:

block-qcow.c: In function `get_cluster_offset':
block-qcow.c:431:3: error: `tmp_ptr' may be used uninitialized in this function
[-Werror=maybe-uninitialized]
   memcpy(tmp_ptr, l1_ptr, 4096);
   ^
block-qcow.c:606:7: error: `tmp_ptr2' may be used uninitialized in this
function [-Werror=maybe-uninitialized]
   if (write(s->fd, tmp_ptr2, 4096) != 4096) {
       ^
cc1: all warnings being treated as errors
/home/dario/Sources/xen/xen/xen.git/tools/blktap2/drivers/../../../tools/Rules.mk:89:
 recipe for target 'block-qcow.o' failed
make[5]: *** [block-qcow.o] Error 1

The proper behavior is to return upon allocation failure.
About what to return, 0 seems the best option, looking
at both the function and the call sites.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Backport-requested-by: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 845e8c1653)
(cherry picked from commit e003d429d8)
2016-01-06 11:34:24 +00:00
Ian Jackson 1c0ac70ad5 libxl: Fix bootloader-related virtual memory leak on pv build failure
The bootloader may call libxl__file_reference_map(), which mmap's the
pv_kernel and pv_ramdisk into process memory.  This was only unmapped,
however, on the success path of libxl__build_pv().  If there were a
failure anywhere between libxl_bootloader.c:parse_bootloader_result()
and the end of libxl__build_pv(), the calls to
libxl__file_reference_unmap() would be skipped, leaking the mapped
virtual memory.

Ideally this would be fixed by adding the unmap calls to the
destruction path for libxl__domain_build_state.  Unfortunately the
lifetime of the libxl__domain_build_state is opaque, and it doesn't
have a proper destruction path.  But, the only thing in it that isn't
from the gc are these bootloader references, and they are only ever
set for one libxl__domain_build_state, the one which is
libxl__domain_create_state.build_state.

So we can clean up in the exit path from libxl__domain_create_*, which
always comes through domcreate_complete.

Remove the now-redundant unmaps in libxl__build_pv's success path.

This is XSA-160.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Tested-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
(cherry picked from commit 62dc4c1a96)
2015-12-08 15:32:13 +00:00
Ian Jackson e875e0e5fc libxl: adjust PoD target by memory fudge, too
PoD guests need to balloon at least as far as required by PoD, or risk
crashing.  Currently they don't necessarily know what the right value
is, because our memory accounting is (at the very least) confusing.

Apply the memory limit fudge factor to the in-hypervisor PoD memory
target, too.  This will increase the size of the guest's PoD cache by
the fudge factor LIBXL_MAXMEM_CONSTANT (currently 1Mby).  This ensures
that even with a slightly-off balloon driver, the guest will be
stable even under memory pressure.

There are two call sites of xc_domain_set_pod_target that need fixing:

The one in libxl_set_memory_target is straightforward.

The one in xc_hvm_build_x86.c:setup_guest is more awkward.  Simply
setting the PoD target differently does not work because the various
amounts of memory during domain construction no longer match up.
Instead, we adjust the guest memory target in xenstore (but only for
PoD guests).

This introduces a 1Mby discrepancy between the balloon target of a PoD
guest at boot, and the target set by an apparently-equivalent `xl
mem-set' (or similar) later.  This approach is low-risk for a security
fix but we need to fix this up properly in xen.git#staging and
probably also in stable trees.

This is XSA-153.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
(cherry picked from commit 56fb5fd623)
(cherry picked from commit 423d2cd814)

Conflicts:
	tools/libxl/libxl_dom.c
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 73b70e3c5d)
2015-10-29 15:14:34 +00:00
Ian Jackson 85ca813ec2 tools/console: Fix build after "xenconsole tolerate tty errors"
2ddcdd96 "tools/console: xenconsole tolerate tty errors" contains a
compile error introduced during conflict resolution of the backport
from 4.4 to 4.3:

 client/main.c: In function 'get_pty_fd':
 client/main.c:124:10: error: passing argument 1 of 'warn' makes pointer from integer without a cast [-Werror]
 In file included from client/main.c:35:0:
 /usr/include/err.h:35:13: note: expected 'const char *' but argument is of type 'int'

Fix this.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
2015-10-21 15:10:22 +01:00
Ian Campbell 634a5d52d6 tools: libxl: allow permissive qemu-upstream pci passthrough.
Since XSA-131 qemu-xen now restricts access to PCI cfg by default. In
order to allow local configuration of the existing libxl_device_pci
"permissive" flag needs to be plumbed through via the new QMP property
added by the XSA-131 patches.

Versions of QEMU prior to XSA-131 did not support this permissive
property, so we only pass it if it is true. Older versions only
supported permissive mode.

qemu-xen-traditional already supports the permissive mode setting via
xenstore.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
(cherry picked from commit 015a373351)
2015-10-20 17:57:28 +01:00
Ian Jackson 2ddcdd96a0 tools/console: xenconsole tolerate tty errors
Since 28d386fc43 (XSA-57), libxl writes an empty value for the
console tty node, with read-only permission for the guest, when
setting up pv console "frontends".  (The actual tty value is later set
by xenconsoled.)   Writing an empty node is not strictly necessary to
stop the frontend from writing dangerous values here, but it is a good
belt-and-braces approach.

Unfortunately this confuses xenconsole.  It reads the empty value, and
tries to open it as the tty.  xenconsole then exits.

Fix this by having xenconsole treat an empty value the same way as no
value at all.

Also, make the error opening the tty be nonfatal: we just print a
warning, but do not exit.  I think this is helpful in theoretical
situations where xenconsole is racing with libxl and/or xenconsoled.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
CC: George Dunlap <george.dunlap@eu.citrix.com>

---
v2: Combine two conditions and move the free
(cherry picked from commit 39ba2989b1)
(cherry picked from commit 7b161be2e5)

Conflicts:
	tools/console/client/main.c
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
2015-10-20 17:53:54 +01:00
Stefano Stabellini 998424e33d libxl: handle read-only drives with qemu-xen
The current libxl code doesn't deal with read-only drives at all.

Upstream QEMU and qemu-xen only support read-only cdrom drives: make
sure to specify "readonly=on" for cdrom drives and return error in case
the user requested a non-cdrom read-only drive.

This is XSA-142, discovered by Lin Liu
(https://bugzilla.redhat.com/show_bug.cgi?id=1257893).

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Backport to Xen 4.5 and earlier, apropos of report and review from
Michael Young.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
2015-09-22 16:56:35 +01:00
Anthony PERARD 6d81366482 libxl: Increase device model startup timeout to 1min.
On a busy host, QEMU may take more than 10s to load and start.

This is likely due to a bug in Linux where the I/O subsystem sometime
produce high latency under load and result in QEMU taking a long time to
load every single dynamic libraries.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 9acfbe14d7)

Conflicts:
	tools/libxl/libxl_internal.h
(cherry picked from commit bbbd29a25d)
(cherry picked from commit fe66a76a2d)
2015-09-18 16:29:55 +01:00
Wei Liu f97021eb92 xl: correct handling of extra_config in main_cpupoolcreate
Don't dereference extra_config if it's NULL. Don't leak extra_config in
the end.

Also fixed a typo in error string while I was there.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 705c9e1242)
(cherry picked from commit ffb4e6387f)
(cherry picked from commit 213e243819)
2015-09-11 11:56:56 +01:00
Matthew Daley 8d394a4355 libxl: fix leak of config_data in main_cpupoolcreate
Coverity-ID: 1087193
Signed-off-by: Matthew Daley <mattjd@gmail.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
(cherry picked from commit 7737ecb20c)
2015-09-11 11:56:51 +01:00
Ian Jackson 3bcb2c062a libxl: poll: Avoid fd deregistration race POLLNVAL crash
It can happen that an fd is deregistered, and closed, and then a new
fd opened, and reregistered, all while another thread is in poll().

If this happens poll might report POLLNVAL, but the event loop would
think that the fd was supposed to have been valid, and then fail an
assertion:
  libxl_event.c:1183: afterpoll_check_fd: Assertion `poller->fds_changed || !(fds[slot].revents & 0x020)' failed.

We can't simply ignore POLLNVAL because if we have bugs which cause
messed-up fds, it is a serious problem which we really need to detect.

Instead, add extra tracking to spot when this possibility arises, and
abort on POLLNVAL if we are sure that it is unexpected.

Reported-by: Jim Fehlig <jfehlig@suse.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
(cherry picked from commit 681ce16816)

(cherry picked from commit 7f7642f778)

And, adjusted for semantic conflict over CTX vs ctx.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

(cherry picked from commit 3646b134c1)
Conflicts:
	tools/libxl/libxl_event.c
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
2015-08-12 12:54:29 +01:00
Ian Jackson f66f746df7 libxl: make libxl__poller_put tolerate p==NULL
This is less fragile, and more in keeping with the usual style of
initialising everything to 0 and freeing things unconditionally.

Correspondingly, remove the tests at the call sites.

Apropos of c1f3f174.  No overall functional change.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
(cherry picked from commit 6ed09e3772)
2015-08-12 12:50:06 +01:00
Ian Jackson 856f59437b libxl: poll: Use poller_get and poller_put for poller_app
This makes the code more regular.  We are going to want to do some
more work in poller_get and poller_put, which work also wants to be
done for poller_app.

Two very minor functional changes:

 * We call malloc an extra time since poller_app is now a pointer

 * ERROR_FAIL on poller_get failing for poller_app is generated in
   libxl_ctx_init rather than passed through by libxl_poller_init
   from libxl__pipe_nonblock.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
(cherry picked from commit aae3765206)

(cherry picked from commit 9f6f513eec)
Conflicts:
	tools/libxl/libxl.c
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 0348c450fe)
2015-08-12 12:43:27 +01:00
Ian Jackson dd80ee80c9 libxl: poll: Make libxl__poller_get have only one success return path
In preparation for doing some more work on successful exit.

No functional change.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
(cherry picked from commit 6fc946bc55)

(cherry picked from commit 8c409135e6)
Conflicts:
	tools/libxl/libxl_event.c
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 900c7970de)
2015-08-12 12:39:36 +01:00
Matthew Daley e9a0fc5998 libxl: don't leak memory in libxl__poller_get failure case
Coverity-ID: 1055894
Signed-off-by: Matthew Daley <mattjd@gmail.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
(cherry picked from commit 1edd6d8da3)
2015-08-12 12:39:10 +01:00
Ian Jackson 32f82fa8d9 libxl: event handling: ao_inprogress does waits while reports outstanding
libxl__ao_inprogress needs to check (like
libxl__ao_complete_check_progress_reports) that there are no
oustanding progress callbacks.

Otherwise it might happen that we would destroy the ao while another
thread has an outstanding callback its egc report queue.  The other
thread would then, in its egc_run_callbacks, touch the destroyed ao.

Instead, when this happens in libxl__ao_inprogress, simply run round
the event loop again.  The thread which eventually makes the callback
will spot our poller in the ao, and notify the poller, waking us up.

This fixes an assertion failure race seen with libvirt:
  libvirtd: libxl_event.c:1792: libxl__ao_complete_check_progress_reports: Assertion `ao->in_initiator' failed.
or (after "Add an assert to egc_run_callbacks")
  libvirtd: libxl_event.c:1338: egc_run_callbacks: Assertion `aop->ao->magic == 0xA0FACE00ul' failed.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
(cherry picked from commit f1335f0d7b)
(cherry picked from commit dfed6d96fd)
2015-08-12 12:25:32 +01:00
Ian Jackson 546960ca05 libxl: event handling: Break out ao_work_outstanding
Break out the test in libxl__ao_complete_check_progress_reports, into
ao_work_outstanding, which reports false if either (i) the ao is still
ongoing or (ii) there is a progress report (perhaps on a different
thread's callback queue) which has yet to be reported to the
application.

No functional change.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
(cherry picked from commit 93699882d9)
(cherry picked from commit ba683105b5)
2015-08-12 12:25:13 +01:00
Konrad Rzeszutek Wilk 116bbc6062 libxl: In libxl_set_vcpuonline check for maximum number of VCPUs against the cpumap.
There is no sense in trying to online (or offline) CPUs when the size of
cpumap is greater than the maximum number of VCPUs the guest can go to.

As such fail the operation if the count of CPUs to online is greater
than what the guest started with. For the offline case we do not
check (as the bits are unset in the cpumap) and let it go through.

We coalesce some of the underlying libxl_set_vcpuonline code
together which was duplicated in QMP and XenStore codepaths.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

(cherry picked from commit d83bf9d224)
Conflicts:
	tools/libxl/libxl.c
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

(cherry picked from commit 0d8cbcad03)
Conflicts:
	tools/libxl/libxl.c
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

(cherry picked from commit ca0f468192)
Conflicts:
	tools/libxl/libxl.c
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
2015-08-12 12:15:20 +01:00
Andrew Cooper c776b6429b tools/libxc: Fix build of 32bit toolstacks on CentOS 5.x following XSA-125
gcc 4.1 of CentOS 5.x era does not like the typecheck in min() between
uint64_t and unsigned long.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>

(cherry picked from commit 6c1cb3dba4)
Conflicts:
	tools/libxc/xc_domain.c
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
2015-08-12 12:04:09 +01:00
Andrew Cooper 9d6754dc2d tools/xenconsoled: Increase file descriptor limit
XenServer's VM density testing uncovered a regression when moving from
sysvinit to systemd where the file descriptor limit dropped from 4096 to
1024. (XenServer had previously inserted a ulimit statement into its
initscripts.)

One solution is to use LimitNOFILE=4096 in xenconsoled.service to match the
lost ulimit, but that is only a stopgap solution.

As Xenconsoled genuinely needs a large number of file descriptors if a large
number of domains are running, attempt to increase the limit.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
(cherry picked from commit 588df84c0d)
(cherry picked from commit 9d5b2b0102)
(cherry picked from commit a490f8df8c)
2015-08-12 11:58:40 +01:00
Andrew Cooper 04f8006b66 ocaml/xenctrl: Fix stub_xc_readconsolering()
The Ocaml stub to retrieve the hypervisor console ring had a few problems.

 * A single 32k buffer would truncate a large console ring.
 * The buffer was static and not under the protection of the Ocaml GC lock so
   could be clobbered by concurrent accesses.
 * Embedded NUL characters would cause caml_copy_string() (which is strlen()
   based) to truncate the buffer.

The function is rewritten from scratch, using the same algorithm as the python
stubs, but uses the protection of the Ocaml GC lock to maintain a static
running total of the ring size, to avoid redundant realloc()ing in future
calls.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Dave Scott <dave.scott@eu.citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
Acked-by: David Scott <dave.scott@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
(cherry picked from commit 1a010ca99e)
(cherry picked from commit cfc4c43be1)
(cherry picked from commit c669c24424)
2015-08-12 11:58:33 +01:00
Andrew Cooper dea3fd1e94 ocaml/xenctrl: Make failwith_xc() thread safe
The static error_str[] buffer is not thread-safe, and 1024 bytes is
unreasonably large.  Reduce to 256 bytes (which is still much larger than any
current use), and move it to being a stack variable.

Also, propagate the Noreturn attribute from caml_raise_with_string().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Dave Scott <Dave.Scott@eu.citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
Acked-by: David Scott <dave.scott@citrix.com>
(cherry picked from commit c8945d5161)
(cherry picked from commit 032673c883)
(cherry picked from commit 9702e084d0)
2015-08-12 11:57:40 +01:00
Andrew Cooper 8d0ca8a5e1 ocaml/xenctrl: Check return values from hypercalls
rather than blindly continuing and possibly using negative values.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Dave Scott <dave.scott@eu.citrix.com>
Acked-by: David Scott <dave.scott@citrix.com>
(cherry picked from commit 3380f5b627)
(cherry picked from commit c91ed889ea)
(cherry picked from commit 10a95535b0)
2015-08-12 11:57:18 +01:00
Ian Jackson 0332b3f24b libxl: In domain death search, start search at first domid we want
From: Ian Jackson <Ian.Jackson@eu.citrix.com>

When domain_death_xswatch_callback needed a further call to
xc_domain_getinfolist it would restart it with the last domain it
found rather than the first one it wants.

If it only wants one it will also only ask for one domain.  The result
would then be that it gets the previous domain again (ie, the previous
one to the one it wants), which still doesn't reveal the answer to the
question, and it would therefore loop again.

It's completely unclear to me why I thought it was a good idea to
start the xc_domain_getinfolist with the last domain previously found
rather than the first one left un-confirmed.  The code has been that
way since it was introduced.

Instead, start each xc_domain_getinfolist at the next domain whose
status we need to check.

We also need to move the test for !evg into the loop, we now need evg
to compute the arguments to getinfolist.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reported-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
(cherry picked from commit 4783c99aab)
(cherry picked from commit 0b19348f3c)
(cherry picked from commit 13623d5d8e)
2015-08-12 11:52:25 +01:00