Xenmmap.unmap sets the address to MAP_FAILED in xenmmap_stubs.c. If due to a
bug there were still references to the Xenbus and we attempt to use it then we
crash. Raise an exception instead of crashing.
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Currently, oxenstored checks the length of paths against 1024, then
prepends "/local/domain/$DOMID/" to relative paths. This allows a domU
to create paths which can't subsequently be read by anyone, even dom0.
This also interferes with listing directories, etc.
Define a new oxenstored.conf entry: quota-path-max, defaulting to 1024
as before. For paths that begin with "/local/domain/$DOMID/" check the
relative path length against this quota. For all other paths check the
entire path length.
This ensures that if the domid changes (and thus the length of a prefix
changes) a path that used to be valid stays valid (e.g. after a
live-migration). It also ensures that regardless how the client tries
to access a path (domid-relative or absolute) it will get consistent
results, since the limit is always applied on the final canonicalized
path.
Delete the unused Domain.get_path to avoid it being confused with
Connection.get_path (which differs by a trailing slash only).
Rewrite Util.path_validate to apply the appropriate length restriction
based on whether the path is relative or not. Remove the check for
connection_path being absolute, because it is not guest controlled data.
This is part of XSA-323.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
dbc84d2983 (Xen >= 4.9.0) deleted XS_RESTRICT
from oxenstored, which caused all the following opcodes to be shifted by 1:
reset_watches became off-by-one compared to the C version of xenstored.
Looking at the C code the opcode for reset watches needs:
XS_RESET_WATCHES = XS_SET_TARGET + 2
So add the placeholder `Invalid` in the OCaml<->C mapping list.
(Note that the code here doesn't simply convert the OCaml constructor to
an integer, so we don't need to introduce a dummy constructor).
Igor says that with a suitably patched xenopsd to enable watch reset,
we now see `reset watches` during kdump of a guest in xenstored-access.log.
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Tested-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
The OCaml code has been fixed to handle properly -safe-string in Xen
4.11, however the stubs part were missed.
On OCaml newer than 4.06.1, String_Val() will return a const char *
when using -safe-string leading to build failure when this is used
in place where char * is expected.
The main use in Xen code base is when a new string is allocated. The
suggested approach by the OCaml community [1] is to use the helper
caml_alloc_initialized_string() but it was introduced by OCaml 4.06.1.
The next best approach is to cast String_val() to (char *) as the helper
would have done. So use it when we need to update the new string using
memcpy().
Take the opportunity to remove the unnecessary cast of the source as
mempcy() is expecting a void *.
[1] https://github.com/ocaml/ocaml/pull/1274
Reported-by: Dario Faggioli <dfaggioli@suse.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Commit ec7d54dd1a "ocaml/libs/xb: Use bytes in place of strings for
mutable buffers" switch mutable buffers from string to bytes. However
the C code were still using String_Val() to access them.
While the underlying structure is the same between string and bytes, a
string is meant to be immutable. OCaml 4.06.1 and later will enforce it.
Therefore, it will not be possible to build the OCaml libs when using
-safe-string. This is because String_val() will return a const value.
To avoid plain cast in the code, the code is now switched to use
Bytes_val(). As the macro is not defined in older OCaml version, we need
to provide a stub.
Take the opportunity to switch to const the buffer in
ml_interface_write() as it should not be modified.
Reported-by: Dario Faggioli <dfaggioli@suse.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
stub_header_of_string() should not modify the header. So mark the
variable 'hdr' as const.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
This commit cleans up code to reduce compiler warnings:
* remove unused open statements (warning 33)
* remove unused 'rec' declarations (warnign 39)
* remove unused type declarations (warning 34)
* mark unused variables with an underscore (warning 27)
* mark unused value declarations with an underscore (warning 32)
This commit does not include changes to fix compiler warnings 52
(matching against strings in exceptions). These changes have no impact
on functionality.
Signed-off-by: Christian Lindig <christian.lindig@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Since Ocaml 4.06.0, that made safe-string on by default, the compiler is
allowed to perform optimisations on immutable strings. They should no
longer be used as mutable buffers, and bytes should be used instead.
The C stubs for Xs_ring have been updated to use bytes, and the interface
rationalised mimicking the new Unix module in the standard library (the
implementation of Unix.write_substring uses unsafe_of_string in the exact same
way, and both the write implementations are using the bytes as an immutable
payload for the write).
Signed-off-by: Marcello Seri <marcello.seri@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
When xenstore was ported to the new safe-string interface, it mostly
happened by making copyies of string into bytes and back. The ideal
fix would be to rewrite all of the relevant interfaces to be uniformly
using bytes, but in the meanwhile we can improve the code by using unsafe
conversion functions (see
https://caml.inria.fr/pub/docs/manual-ocaml/libref/Bytes.html#3_Unsafeconversionsforadvancedusers).
In most cases we own the bytes that we are converting to string, or we
immediately make copies that we then mutate, or we use them immutably
as payloads for writes. In all these cases it is safe to use the unsafe
functions and prevent a copy.
This patch updates the code to use the unsafe conversions where possible.
Signed-off-by: Marcello Seri <marcello.seri@citrix.com>
Reviewed-by: Christian Lindig <christian.lindig@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Xen built with ocaml 4.06 gives errors such as
Error: This expression has type bytes but an expression was
expected of type string
as Byte and safe-strings which were introduced in 4.02 are the
default in 4.06.
This patch which is partly by Richard W.M. Jones of Red Hat
from https://bugzilla.redhat.com/show_bug.cgi?id=1526703
fixes these issues.
Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
Reviewed-by: Christian Lindig<christian.lindig@citrix.com>
To stay in line with other parts of the ocaml code base.
This requires committing a bunch of mli files in tree.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Xen built with ocaml 4.06 gives errors such as
Error: This expression has type bytes but an expression was
expected of type string
as Byte and safe-strings which were introduced in 4.02 are the
default in 4.06.
This patch which is mostly by Richard W.M. Jones of Red Hat
from https://bugzilla.redhat.com/show_bug.cgi?id=1526703
fixes these issues.
v2: drop tools/ocaml/libs/xc/xenctrl.ml from the patch as the
affected code was removed by commit d933f1a53c
tools/ocaml: Drop coredump infrastructure
Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
Reviewed-by: Christian Lindig <christian.lindig@citrix.com>
[ wei: remove trailing whitespaces ]
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
XS_RESTRICT and the xenstore library function xs_restrict() have never
been usable in all configurations and there are no known users.
This functionality was thought to limit access rights of device models
to xenstore in order to avoid affecting other domains in case of a
security breech. Unfortunately XS_RESTRICT won't help as current
qemu is requiring access to dom0 only accessible xenstore paths to
work correctly. So this command is useless and should be removed.
In order to avoid problems in the future remove all support for
XS_RESTRICT from xenstore.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: David Scott <dave@recoil.org>
ml_interface_{read,write}() would miscalculate the quantity of
data/space in the ring if it crossed the ring boundary, and incorrectly
return a short read/write.
This causes a protocol stall, as either side of the ring ends up waiting
for what they believe to be the other side needing to take the next
action.
Correct the calculations to cope with crossing the ring boundary.
In addition, correct the error detection. It is a hard error if the
producer index gets more than a ring size ahead of the consumer, or if
the consumer ever overtakes the producer.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: David Scott <dave@recoil.org>
In 674ad2b (xenstore: extend the xenstore ring with a 'closing' signal)
two uses of uint32 are added to tools/ocaml/libs/xb/xs_ring_stubs.c .
As of ocaml 4.03.0+dev the uint32 type is no longer supported. This patch
replaces the uses of uint32 with uint32_t .
Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
Acked-by: David Scott <dave.scott@citrix.com>
Hvmloader uses the xenstore ring and then tries to reset it back
to its initial state before booting the guest. Occasionally xenstored
will read the ring while it is being zeroed and conclude it has
been corrupted. This prevents PV drivers from loading in the guest.
This patch updates the xenstore ring protocol definition, enabling
a server to advertise additional features to the guest. One such feature
is defined: the ability to cleanly reset the ring including the
higher-level protocol, like an enhanced RESET_WATCHES for rings.
This patch implements the ring reconnection features in oxenstored
and hvmloader, fixing the bug.
This patch also defines an 'invalid' xenstore packet type and uses this
to poison the ring over a reconnect. This will make diagnosing this
bug much easier in future.
Signed-off-by: David Scott <dave.scott@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Jon Ludlam <jonathan.ludlam@citrix.com>
OCaml is replacing its own uint32 type with the ISO C99 uint32_t type
in 4.03.0+dev. Ths patch replaces the remaining uses of uint32 with
uint32_t in the ocaml C code for xen.
Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
Acked-by: David Scott <dave.scott@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>
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>
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.
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>
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>
configure checks just for ocamlc, but the tools in tools/ocaml depend
also on ocamlfind. On my workstation I have just ocamlc installed, but
no ocamlfind. As a result make will fail.
Update configure.ac to check also for OCAMLFIND, update various
Makefiles and replace hardcoded ocamlfind string with $(OCAMLFIND)
Please rerun autogen.sh after applying this patch.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Committed-by: Ian Campbell <ian.campbell@citrix.com>
Previous an unknown operation would be decoded as a Not_found exception which
would bubble all the way up to the try ... with surrounding the call to
main_loop where it would be logged and ignored.
This would leave the guest hanging waiting for a response to the invalid
request.
Instead introduce a specific "Invalid" operation. Higher level functionality,
such as Process.process_packet, already handles operations which are not
understood with an error reply due to the final wildcard entry in
Process.function_of_type but explicitly handle Invalid this way to make it
clear what is going on.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
It is unused.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Ian Jackson <ian.jackson.citrix.com>
Committed-by: Ian Jackson <ian.jackson.citrix.com>
ocamlfind does not support namespaces, so to avoid
name clashes the module names have become longer.
Additionally, the xenstore and xenbus subdirs, which
contain several modules each, have been packed into
toplevel Xenstore and Xenbus modules.
xb becomes xenbus, xc becomes xenctrl, xl becomes xenlight,
xs becomes xenstore, eventchn becomes xeneventchn and
mmap becomes xenmmap.
[ Patch modified from that submitted, to update the .hgignore, and to
cope with intervening changes to mmap_stubs.c -iwj ]
Signed-off-by: Jon Ludlam <jonathan.ludlam@eu.citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
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>
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>
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>
Edwin Török
Julien Grall
Christian Lindig
Marcello Seri
Michael Young
Wei Liu
Juergen Gross
Andrew Cooper
David Scott
Olaf Hering
=John Liu
Ian Campbell
Ian Jackson
Jon Ludlam
Keir Fraser
Keir Fraser