This fixes the issue for both C and ocaml xenstored, however only the ocaml
xenstored is vulnerable in its default configuration.
Adding a new error appears to be safe, since bit libxenstore and the Linux
driver at least treat an unknown error code as EINVAL.
This is XSA-72 / CVE-2013-4416.
Original ocaml patch by Jerome Maloberti <jerome.maloberti@citrix.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
master commit: 8b2c441a1b
master date: 2013-10-29 15:45:53 +0000
This change:
070ab4c505
"oxenstored: Protect oxenstored from malicious domains"
broke the build because it had an unresolved semantic (but not
textual) conflict with
c69fddbd5d
"tools/ocaml: Remove log library from tools/ocaml/libs"
(which is in 4.2 but not 4.1)
Fix this by using the 4.1.x idiom in the new error handling introduced
in 070ab4c505.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Acked-by: David Scott <dave.scott@eu.citrix.com>
add check logic when read from IO ring, and if error happens,
then mark the reading connection as "bad", Unless vm reboot,
oxenstored will not handle message from this connection any more.
xs_ring_stubs.c: add a more strict check on ring reading
connection.ml, domain.ml: add getter and setter for bad flag
process.ml: if exception raised when reading from domain's ring,
mark this domain as "bad"
xenstored.ml: if a domain is marked as "bad", do not handle it.
Signed-off-by: John Liu <john.liuqiming@huawei.com>
Acked-by: David Scott <dave.scott@eu.citrix.com>
(cherry picked from commit 704302ce94)
(cherry picked from commit a978634bee)
Conflicts:
tools/ocaml/xenstored/domain.ml
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 9f93027afd)
Change 26521:2c0fd406f02c (part of XSA-38 / CVE-2013-0215) incorrectly
caused us to ignore rather than process a completely full ring. Check if
producer and consumer are equal before masking to avoid this, since prod ==
cons + PAGE_SIZE after masking becomes prod == cons.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Keir Fraser <keir@xen.org>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
xen-unstable changeset: 26539:759574df84a6
Backport-requested-by: security@xen.org
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
The maximum size of a message is part of the protocol spec in
xen/include/public/io/xs_wire.h
Before this patch a client which sends an overly large message can
cause a buffer read overrun.
Note if a badly-behaved client sends a very large message
then it will be difficult for them to make their connection
work again-- they will probably need to reboot.
This is a security issue, part of XSA-38 / CVE-2013-0215.
Signed-off-by: David Scott <dave.scott@eu.citrix.com>
Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
xen-unstable changeset: 26522:ffd30e7388ad
Backport-requested-by: security@xen.org
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
oxenstored makes use of the OCaml Xenbus bindings, in which the
function xs_ring_read in tools/ocaml/libs/xb/xs_ring_stubs.c is used
to read from the shared memory Xenstore ring.
This function does not correctly handle all possible (prod, cons)
states when MASK_XENSTORE_IDX(prod) > MASK_XENSTORE_IDX(cons).
The root cause is the use of the unmasked values of prod and cons to
calculate to_read. If prod is set to an out-of-range value, the ring
peer can cause to_read to be too large or even negative. This allows
the ring peer to force oxenstored to read and write out of range for
the buffers leading to a crash or possibly to privilege escalation.
Correct this by masking the values of cons and prod at the start, so
we only deal with masked values. This makes the logic simpler, as
semantically inappropriate values of the upper bits of the ring
pointers are simply ignored.
The same vulnerability does not exist in the ring writer because the
only use made of the unmasked value is the check which prevents the
prod pointer overtaking the cons pointer. A ring peer which defeats
this check will suffer only lost data.
However, additionally, precautions need to be taken to ensure that
req_cons and req_prod are only read once in each function. Without
the use of volatile or some asm construct, the compiler can "prove"
that req_cons and req_prod do not change unexpectedly and is permitted
to "amplify" the read of (say) req_cons into two reads at different
times, giving two different values for use as cons, and then use the
two sources of cons interchangeably. (The use of xen_mb() does not
forbid this.)
Therefore do the reads of req_cons and req_prod through a volatile
pointer in both xs_ring_read and xs_ring_write.
This is currently believed to be a theoretical vulnerability as we are
not aware of any compilers which amplify reads in this way.
This is a security issue, part of XSA-38 / CVE-2013-0215.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Tested-by: Matthew Daley <mattjd@gmail.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
xen-unstable changeset: 26521:2c0fd406f02c
Backport-requested-by: security@xen.org
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
The bit bugs are in ocaml vcpu affinity calls, and the off-by-one
error is in the ocaml console ring code
Signed-off-by: Zheng Li <zheng.li@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell.com>
Committed-by: Ian Jackson <ian.jackson.citrix.com>
Acked-by: Jon Ludlam <jonathan.ludlam@eu.citrix.com>
xen-unstable changeset: 23940:187d59e32a58
xen-unstable date: Mon Oct 10 16:41:16 2011 +0100
Otherwise make can search the path relative to certain standard paths
such as /usr/include (e.g., the line '-include $(XEN_ROOT)/.config' in
Config.mk suffers from this).
Signed-off-by: Keir Fraser <keir@xen.org>
xen-unstable changeset: 23049:ff3b7749008b
Backport-requested-by: Allen M Kay <allen.m.kay@intel.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
OCaml libraries will live in /usr/local/ if the user compiles OCaml
from source. This patch asks the OCaml compiler where we should look
for libraries.
NB: it may be that we should do the same thing for the NetBSD case,
but I don't have a BSD box to test this out.
Signed-off-by: Mike McClurg <mike.mcclurg@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
xen-unstable changeset: 23566:7e5b54d1643e
xen-unstable date: Tue Jun 21 18:01:51 2011 +0100
Currently we pile all the backend and format information pertaining to
disk option in a single enum. This check-in separates the two and
uses two enums, one for disk format and another for disk backend.
This helps clearly differentiate between disk format and backend
within the implementation and also helps cleanup the code in this area
in preparation for the impending parser revamping to be done post 4.1.
Along with separating format and backend, this check-in also removes
unwanted types and renames variables in the disk interface and fixes
the code affected by the interface changes.
In specific, here are the disk interface changes made - In
libxl_device_disk structure physpath was renamed to pdev_path,
virtpath was renamed to vdev, phystype was removed and replaced with
backend and format enums. Also previously a single enum
libxl_disk_phystype held the values for qcow, qcow2, vhd, aio, file,
phy, empty and that got refactored into two enums, libxl_disk_format
to hold unknown, qcow, qcow2, vhd, raw, empty and libxl_disk_backend
to hold unknown, phy, tap and qdisk.
Signed-off-by: Kamala Narasimhan <kamala.narasimhan@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
These functions are introduced as the new way to create domains with libxl
they prevent the callers from need to know about low-level implementation
details such as:
- libxl_domain_make()
- libxl_domain_build()
- libxl_domain_restore()
- when to attach the console
- how to start the device model
Above mentioned functions and all API's for the device model, which are now
redundant, have been made internal to libxl and no longer accessible.
The ocaml binding for libxl has not been properly updated to reflect the
changes, wrappers for the old functions have been removed but the code to wrap
the new functions has not been added.
Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Now that tools/libxc is licensed under LGPL I don't think there is any need for
an LGPL reimplementation under tools/ocaml.
For the most part the conversion to the up-to-date libxc API (xc_lib.c
essentially implemented the same interface as an older libxc) was pretty
automatic. There are some functions which appear to no longer exist in libxc
which I therefore simply removed the bindings for and a small number of
interfaces which had changed.
Many of the functions bound by the stubs have no in-tree users (which I think
is fine for a language binding) so I have no way to confirm correctness other
than by eye. I was however able to confirm that oxenstored still worked and to
build a XCP toolstack which could successfully start a PV guest.
Uses the new XC_OPENFLAG_NON_REENTRANT option to avoid potential conflicts
between pthreads and the ocaml runtime.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Vincent Hanquez <Vincent.Hanquez@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Rename Evtchn.bind_virq as Evtchn.bind_dom_exc_virq
to reflect its actual behaviour.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Previously the install target was having no effect because it ended up
calling the default target in the subdir Makefile instead of the
install target.
Resolve this by tying the tools/ocaml Makefiles into the generic
handling done by tools/Rules.mk.
Other changes arising in one way or another from this:
- Add libs/xl/META.in
- Update .hgignore for META files
- Create leading directories
- Remove existing module before installation in install targer
(worksaround what appears to be a quirk of "ocamlfind install")
- Use the globally defined $(DESTDIR)
- Move "ocamlfind printfconf destdir" to a common variable,
repurposing exising unused OCAMLDESTDIR, incorporating $(DESTDIR) at
the same time.
- Drop a few unused variabe definitions (mainly to avoid deciding if
$(DESTDIR) made sense for them or not.
- Pass -destdir to ocamlfind in uninstall target for symmetry with
install target.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Firstly remove an anonymous union in libxl_device_pci structure which
was making auto-generating language bindings more complicated than
necessary and exporting random bits of low level ABI that libxl that
would rather hide anyway. There is a corresponding (untested) change to
the ocaml binding which maintains previous ml API.
Secondly make the libxl_file_reference type a Builtin. This is a
'semantic
correctness' issue in that libxl ABI/API won't change. But it makes it
so that when the IDL is used to generate language bindings that a
file_reference type is not exported.
Also implement a Numeric type which all integers are derived from. Make
sure a boolean signed/unsigned attribute is set accordingly. This is
required to allow language bindings to correctly handle the sign bit in
environments with arbitrarily long integers.
Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
This patch converts xl to randomly generate UUID's rather than using a
dodgy time-seeded PRNG. I have ignored various suggestions so far on
auto-generation of MAC addresses and left it as a topic for a future
patch to solve. In other words the behaviour stays the same it's just
using a true random source. This patch also implements the "uuid" config
file parameter in xl.
Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
Acked-By: Christoph Egger <Christoph.Egger@amd.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
This patch implements the new protocol for handling pv consoles and
emulated serials as described in the document docs/misc/console.txt.
The changes are:
- xenconsoled: do not write the pty under serial in xenstore if
xenconsoled is handling a consolepath;
- xenconsole: implement support for an explicit console type parameter;
the parameter can be "pv", to specify that the user wants to
connect to a pv console, or "serial", to specify that the user wants to
connect to an emulated serial. If the type parameter hasn't been
specified be the user, xenconsole tries to guess which type of console
it has to connect to, defaulting to pv console for pv guests and
emulated serial for hvm guests.
- xenconsole: use the new xenstore paths;
- libxl: rename libxl_console_constype to libxl_console_consback:
constype is used to to specify whether qemu or xenconsoled provides the
backend, so I renamed it to libxl_console_consback to make it more
obvious that we are talking about backends;
- libxl: add a new libxl_console_constype to specify if the console is
an emulated serial or a pv console;
- libxl: support the new xenconsole "type" command line parameter;
- libxl: use the "output" node under console in xenstore to tell qemu
where do we want the output of this pv console to go;
- remove the legacy "serialpath" from xenconsoled altogether
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Also: update the QEMU_TAG to pull in the qemu part of these changes.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
This will allow us to map and then unlink the file and therefore
delete the process on process exit or explicit unmap.
Using the mmaped versions of these files required rewriting build_pv
to use the xc_dom builder functionality directly rather than through
the xc_linux_build "compatibility layer". (The status of the
xc_linux_build interface as a compatibility layer seems a bit dubious
since all existing callers use it but if anything is going to replace
it then libxl seems like the likely candidate).
I'm not thrilled with the definition of the maps lifecycle. This could
be solved by adding a helper function to explicitly free the toplevel
structure.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
The OCaml xenstored supports the XS_RESTRICT operation, which
deprivileges a dom0 xenstore connection so it can only affect one
domain's entries. Add the relevant definitions to the C libraries
so that callers can use it.
Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
tools/ocaml was capable of descending into lib/* and xenstored/
simultaneously, obviously with poor results.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Oxenstored should not try to create the evtchn device, as it:
* creates the wrong name (/dev/xen/eventchn rather than evtchn)
* uses a hard-coded minor number, even though this dynamically
depends on what other misc devices are in the kernel
Remove all this code and just rely on the system to create the device.
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
cc1: warnings being treated as errors
xc_lib.c: In function 'xc_domain_get_pfn_list':
xc_lib.c:1217: error: assignment from incompatible pointer type
The XEN_DOMCTL_getmemlist interface has been 32/64 invariante since
13594:30af6cfdb05c and uint64_t is now the correct type for the PFN
list on all word sizes.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Ian Campbell
Ian Jackson
Zheng Li
Keir Fraser
Mike McClurg
Kamala Narasimhan
Gianni Tedesco
Wei Gang
Vincent Hanquez
Stefano Stabellini
Stefano Stabellini
Keir Fraser