* 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>
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>
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>
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>
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>
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>
* 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>
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>
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>
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>
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>
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>
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)
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)
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)
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>
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)
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)
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>
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)
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)
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)
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)
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>
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)
Various xl sub-commands take additional parameters containing = as
additional config fragments.
The handling of these config fragments has a number of bugs:
1. Use of a static 1024-byte buffer. (If truncation would occur,
with semi-trusted input, a security risk arises due to quotes
being lost.)
2. Mishandling of the return value from snprintf, so that if
truncation occurs, the to-write pointer is updated with the
wanted-to-write length, resulting in stack corruption. (This is
XSA-137.)
3. Clone-and-hack of the code for constructing the appended
config file.
These are fixed here, by introducing a new function
`string_realloc_append' and using it everywhere. The `extra_info'
buffers are replaced by pointers, which start off NULL and are
explicitly freed on all return paths.
The separate variable which will become dom_info.extra_config is
abolished (which involves moving the clearing of dom_info).
Additional bugs I observe, not fixed here:
4. The functions which now call string_realloc_append use ad-hoc
error returns, with multiple calls to `return'. This currently
necessitates multiple new calls to `free'.
5. Many of the paths in xl call exit(-rc) where rc is a libxl status
code. This is a ridiculous exit status `convention'.
6. The loops for handling extra config data are clone-and-hacks.
7. Once the extra config buffer is accumulated, it must be combined
with the appropriate main config file. The code to do this
combining is clone-and-hacked too.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Tested-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Ian Campbell <ian,campbell@citrix.com>
(cherry picked from commit dd84604f35)
(cherry picked from commit 6040b3aeb3)
(cherry picked from commit 214fd40a20)
By default qemu will try to create some sort of backend for the
emulated VGA device, either SDL or VNC.
However when the user specifies sdl=0 and vnc=0 in their configuration
libxl was not explicitly disabling either backend, which could lead to
one unexpectedly running.
If either sdl=1 or vnc=1 is configured then both before and after this
change only the backends which are explicitly enabled are configured,
i.e. this issue only occurs when all backends are supposed to have
been disabled.
This affects qemu-xen and qemu-xen-traditional differently.
If qemu-xen was compiled with SDL support then this would result in an
SDL window being opened if $DISPLAY is valid, or a failure to start
the guest if not. Passing "-display none" to qemu before any further
-sdl options disables this default behaviour and ensures that SDL is
only started if the libxl configuration demands it.
If qemu-xen was compiled without SDL support then qemu would instead
start a VNC server listening on ::1 (IPv6 localhost) or 127.0.0.1
(IPv4 localhost) with IPv6 preferred if available. Explicitly pass
"-vnc none" when vnc is not enabled in the libxl configuration to
remove this possibility.
qemu-xen-traditional would never start a vnc backend unless asked.
However by default it will start an SDL backend, the way to disable
this is to pass a -vnc option. In other words passing "-vnc none" will
disable both vnc and sdl by default. sdl can then be reenabled if
configured by subsequent use of the -sdl option.
Tested with both qemu-xen and qemu-xen-traditional built with SDL
support and:
xl cr # defaults
xl cr sdl=0 vnc=0
xl cr sdl=1 vnc=0
xl cr sdl=0 vnc=1
xl cr sdl=0 vnc=0 vga=\"none\"
xl cr sdl=0 vnc=0 nographic=1
with both valid and invalid $DISPLAY.
This is XSA-119 / CVE-2015-2152.
Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 91b0ae9db3)
(cherry picked from commit 6616c4d6fe)
(cherry picked from commit 84ca072f37)
libxl__device_disk_local_initiate_attach is assigning dls->diskpath with a
strdup of the device path. This is then passed to the callback, e.g.
parse_bootloader_result but bootloader_cleanup will not free it.
Since the callback is within the scope of the (e)gc and therefore doesn't need
to be malloc'd, a gc'd alloc will do. All other assignments to this field use
the gc.
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=767295
Reported-by: Gedalya <gedalya@gedalya.net>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
(cherry picked from commit 379b351889)
Valgrind reports:
==7971== Invalid read of size 1
==7971== at 0x40877BE: libxl__parse_mac (libxl_internal.c:288)
==7971== by 0x405C5F8: libxl__device_nic_from_xs_be (libxl.c:3405)
==7971== by 0x4065542: libxl__append_nic_list_of_type (libxl.c:3484)
==7971== by 0x4065542: libxl_device_nic_list (libxl.c:3504)
==7971== by 0x406F561: libxl_retrieve_domain_configuration (libxl.c:6661)
==7971== by 0x805671C: reload_domain_config (xl_cmdimpl.c:2037)
==7971== by 0x8057F30: handle_domain_death (xl_cmdimpl.c:2116)
==7971== by 0x8057F30: create_domain (xl_cmdimpl.c:2580)
==7971== by 0x805B4B2: main_create (xl_cmdimpl.c:4652)
==7971== by 0x804EAB2: main (xl.c:378)
This is because on the final iteration the tok += 3 skips over the terminating
NUL to the next byte, and then *tok reads it. Fix this by using endptr as the
iterator.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Reviewed-by: Don Slutz <dslutz@verizon.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
(cherry picked from commit 5a430eca0b)
On a system which looks like this:
[root@st04 ~]# xl list
Name ID Mem VCPUs State Time(s)
Domain-0 0 752 4 r----- 46699.3
(null) 1 0 0 --p--- 0.0
(null) 2 0 0 --p--- 0.0
(null) 3 0 0 --p--- 0.0
badger 25 0 1 --p--- 0.0
`xl vcpu-list` failes as so:
[root@st04 ~]# xl vcpu-list
Name ID VCPU CPU State Time(s) CPU Affinity
Domain-0 0 0 0 -b- 12171.0 all
Domain-0 0 1 1 -b- 11779.6 all
Domain-0 0 2 2 -b- 11599.0 all
Domain-0 0 3 3 r-- 11007.0 all
libxl: critical: libxl__calloc: libxl: FATAL ERROR: memory allocation failure (libxl__calloc, 4294935299 x 40)
: Cannot allocate memory
libxl: FATAL ERROR: memory allocation failure (libxl__calloc, 4294935299 x 40)
The root cause of this is in Xen. getdomaininfo() has no way of expressing
"this domain has no vcpus". Previously, info->max_vcpu_id would be returned
uninitialised in such a case.
Unfortunately, setting it to 0 as a default is not appropriate. A max_vcpu_id
of 0 and nr_online_cpus of 0 is the valid state for a single vcpu domain which
is in the process of being destroyed.
As all components are required to add 1 to max_vcpu_id to get the number of
vcpus, an id of ~0U is not valid to be used. Explicitly define this as an
invalid max vcpu value, and use it to express "no vcpus" in getdomaininfo()
In libxl, the issue is seen as libxl_list_vcpu() attempts to use the
uninitialised domaininfo.max_vcpu_id for memory allocation.
Check domaininfo.max_vcpu_id against the new sentinel value
XEN_INVALID_MAX_VCPU_ID, and return early. This means that it is now valid
for libxl_list_vcpu() to return NULL for a domain which lacks any vcpus.
As part of this change, remove the pointless call to libxl_get_max_cpus(),
whose returned value is unconditionally clobbered in the for() loop.
Reported-by: Euan Harris <euan.harris@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Don Slutz <dslutz@verizon.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
tools/libxl: Fix libxl_list_vcpu() following c/s 93e52d52
My reasoning regarding nr_cpus_out was wrong, as I had confused nr_cpus_out
with nr_vcpus_out.
Dario pointed this out, but the patch (having gained appropriate acks) got
committed before I could post a correction.
Noticed-by: Dario Faggioli <dario.faggioli@citrix.com>
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>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
master commit: 93e52d5242
master date: 2014-10-23 10:19:53 +0200
master commit: c90a755f26
master date: 2014-10-23 12:29:00 +0100
It is very common for BIOSes to advertise more cpus than are actually present
on the system, and mark some of them as offline. This is what Xen does to
allow for later CPU hotplug, and what BIOSes common to multiple different
systems do to to save fully rewriting the MADT in memory.
An excerpt from `xl info` might look like:
...
nr_cpus : 2
max_cpu_id : 3
...
Which shows 4 CPUs in the MADT, but only 2 online (as this particular box is
the dual-core rather than the quad-core SKU of its particular brand)
Because of the way Xen exposes this information, a libxl_cputopology array is
bounded by 'nr_cpus', while cpu bitmaps are bounded by 'max_cpu_id + 1'.
The current libxl code has two places which erroneously assume that a
libxl_cputopology array is as long as the number of bits found in a cpu
bitmap, and valgrind complains:
==14961== Invalid read of size 4
==14961== at 0x407AB7F: libxl__get_numa_candidate (libxl_numa.c:230)
==14961== by 0x407030B: libxl__build_pre (libxl_dom.c:167)
==14961== by 0x406246F: libxl__domain_build (libxl_create.c:371)
...
==14961== Address 0x4324788 is 8 bytes after a block of size 24 alloc'd
==14961== at 0x402669D: calloc (in/usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==14961== by 0x4075BB9: libxl__zalloc (libxl_internal.c:83)
==14961== by 0x4052F87: libxl_get_cpu_topology (libxl.c:4408)
==14961== by 0x407A899: libxl__get_numa_candidate (libxl_numa.c:342)
...
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
(cherry picked from commit 81b0305048)
(cherry picked from commit 3aaa40fd58)
If libxl_pipe fails we leave the carefd locked, which translates to
the atfork lock remaining held. This would probably cause the process
to deadlock shortly afterwards.
Of course libxl_pipe is very unlikely to fail unless things are
already going very badly. This bug has not been observed anywhere as
far as we are aware.
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>
(cherry picked from commit 7eb73add5d)
(cherry picked from commit 4bb3a17449)
This avoids the process being forked while a carefd is recorded in the
list but the actual fd has been closed. If that happened, a
subsequent libxl_postfork_child_noexec would attempt to close the fd
again. If we are lucky that results in a harmless warning; but if we
are unlucky the fd number has been reused and we close an unrelated
fd.
This race has not been observed anywhere as far as we are aware.
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>
(cherry picked from commit 2a0c3a62ea)
(cherry picked from commit 86c00cb6e2)
This will change initiate_domain_create to walk through NIC definitions
and automatically assign devids to those which have not assigned one.
The devids are needed later in domcreate_launch_dm (for HVM domains
using emulated NICs). The command string for starting the device-model
has those ids as part of its arguments.
Assignment of devids in the hotplug case is handled by libxl_device_nic_add
but that would be called too late in the startup case.
I also moved the call to libxl__device_nic_setdefault here as this seems
to be the only path leading there and avoids doing the loop a third time.
The two loops are trying to handle a case where the caller sets some devids
(not sure that should be valid) but leaves some unset.
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 2d03be65d5)
The current behavior is:
42sh> xl uptime
'xl uptime' requires at least 1 argument.
Usage: xl [-v] uptime [-s] [Domain]
The normal behavior should list uptime for each domain when there is no
parameters.
Signed-off-by: Julien Grall <julien.grall@linaro.org>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 3533972f6d)
If initializing the ctx's lock fails, don't keep going, but instead
error out.
Coverity-ID: 1055289
Signed-off-by: Matthew Daley <mattd@bugfuzz.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit b1cb2bdde1)
Checking the logfile_w fd for -1 on failure is no longer true, because
libxl__create_qemu_logfile will now return ERROR_FAIL on failure which
is -3.
While there also add an error check for opening /dev/null.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
(cherry picked from commit 3b88d95e9c)
Conflicts:
tools/libxl/libxl_dm.c
Coverity ID: 1055290
Calling LIBXL__LOG_ERRNO(ctx,) with a ctx pointer we have just failed to
allocate is going to end badly. Opencode a suitable use of xtl_log() instead.
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>
(cherry picked from commit 1677af03c1)
If the named path is a leaf node, libxl__xs_directory can succeed,
returning non-null, but set *nb to 0.
In three places in libxl this may result in a zero size argument being
passed to malloc() or realloc(), which is not adviseable.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
(cherry picked from commit 48249a1403)
libxl__datacopier_prefixdata would prepend a deliberately short block
(not just a half-full one, but one with a short buffer) to the
dc->bufs queue. However, this is wrong because datacopier_readable
will find it and try to continue to fill it up.
Instead, allocate a full-sized buffer.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Tested-by: Chunyan Liu <cyliu@suse.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
(cherry picked from commit f6d2a87f05)
Ian Jackson
Ian Campbell
Stefano Stabellini
Anthony PERARD
Wei Liu
Matthew Daley
Konrad Rzeszutek Wilk
Julien Grall
Andrew Cooper
Stefan Bader
Matthew Daley
Roger Pau Monne