Commit Graph
401 Commits
Author SHA1 Message Date
Roger Pau Monné 0570d7f276 x86/msr: introduce an option for compatible MSR behavior selection
Introduce an option to allow selecting a behavior similar to the pre
Xen 4.15 one for accesses to MSRs not explicitly handled. Since commit
84e848fd7a and 322ec7c89f accesses to MSRs not explicitly
handled by Xen result in the injection of a #GP to the guest. This
is a behavior change since previously a #GP was only injected if
accessing the MSR on the real hardware would also trigger a #GP, or if
the attempted to be set bits wouldn't match the hardware values (for
PV). The reasons for not leaking hardware MSR values and injecting a
#GP are fully valid, so the solution proposed here should be
considered a temporary workaround until all the required MSRs are
properly handled.

This seems to be problematic for some guests, so introduce an option
to fallback to this kind of legacy behavior without leaking the
underlying MSR values to the guest.

When the option is set, for both PV and HVM don't inject a #GP to the
guest on MSR read if reading the underlying MSR doesn't result in a
#GP, do the same for writes and simply discard the value to be written
on that case.

Note that for guests restored or migrated from previous Xen versions
the option is enabled by default, in order to keep a compatible
MSR behavior. Such compatibility is done at the libxl layer, to avoid
higher-level toolstacks from having to know the details about this flag.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Reviewed-by: Ian Jackson <iwj@xenproject.org>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
2021-03-12 08:59:56 +01:00
Edwin Török 45dee7d92b tools/oxenstored: mkdir conflicts were sometimes missed
Due to how set_write_lowpath was used here it didn't detect create/delete
conflicts.  When we create an entry we must mark our parent as modified
(this is what creating a new node via write does).

Otherwise we can have 2 transactions one creating, and another deleting a node
both succeeding depending on timing.  Or one transaction reading an entry,
concluding it doesn't exist, do some other work based on that information and
successfully commit even if another transaction creates the node via mkdir
meanwhile.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
2021-02-04 14:17:44 +00:00
Edwin Török dc8caf214f tools/oxenstored: Reject invalid watch paths early
Watches on invalid paths were accepted, but they would never trigger.  The
client also got no notification that its watch is bad and would never trigger.

Found again by the structured fuzzer, due to an error on live update reload:
the invalid watch paths would get rejected during live update and the list of
watches would be different pre/post live update.

The testcase is watch on `//`, which is an invalid path.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
2021-02-04 14:17:44 +00:00
Edwin Török c8b9670825 tools/oxenstored: Fix quota calculation for mkdir EEXIST
We increment the domain's quota on mkdir even when the node already exists.
This results in a quota inconsistency after live update, where reconstructing
the tree from scratch results in a different quota.

Not a security issue because the domain uses up quota faster, so it will only
get a Quota error sooner than it should.

Found by the structured fuzzer.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Release-Acked-by: Ian Jackson <iwj@xenproject.org>
2021-02-04 14:17:44 +00:00
Andrew Cooper a48066d25c xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vmtrace
We're about to introduce support for Intel Processor Trace, but similar
functionality exists in other platforms.

Aspects of vmtrace can reasonably can be common, so start with
XEN_SYSCTL_PHYSCAP_vmtrace and plumb the signal from Xen all the way down into
`xl info`.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2021-01-30 03:21:33 +00:00
Edwin Török b1278939db tools/oxenstored: Use more efficient tries
No functional change, just an optimization.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2021-01-22 18:01:35 +00:00
Edwin Török 3068dfd641 tools/oxenstored: Use more efficient node trees
This changes the output of xenstore-ls to be sorted.  Previously the keys were
listed in the order in which they were inserted in.  docs/misc/xenstore.txt
doesn't specify in what order keys are listed.

Map.update is used to retain semantics with replace_child: only an existing
child is replaced, if it wasn't part of the original map we don't add it.
Similarly exception behaviour is retained for del_childname and related
functions.

Entries are stored in reverse sort order, so that upon Map.fold the
constructed list is sorted in ascending order and there is no need for a
List.rev.

This changes the semantics and is not suitable as is for a backport.  It
reveals bugs in buggy clients that depend on xenstore entry order, however
those clients should be fixed.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2021-01-22 18:01:35 +00:00
Edwin Török faf02c345e tools/oxenstored: Backport find_opt/update from 4.06
We are currently on OCaml 4.02 as minimum version.  To make the followup
optimizations compile backport these functions from OCaml 4.06.

This implementation is less efficient than the one in the 4.06 standard
library which has access to the internals of the Map.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2021-01-22 18:01:34 +00:00
Edwin Török 5e115dcf76 tools/oxenstored: Replace hand rolled GC with weak GC references
The code here is attempting to reduce memory usage by sharing common
substrings in the tree: it replaces strings with ints, and keeps a string->int
map that gets manually garbage collected using a hand-rolled mark and sweep
algorithm.

This is unnecessary: OCaml already has a mark-and-sweep Garbage Collector
runtime, and sharing of common strings in tree nodes can be achieved through
Weak references: if the string hasn't been seen yet it gets added to the Weak
reference table, and if it has we use the entry from the table instead, thus
storing a string only once.  When the string is no longer referenced OCaml's
GC will drop it from the weak table: there is no need to manually do a
mark-and-sweep, or to tell OCaml when to drop it.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2021-01-22 18:01:34 +00:00
Edwin Török 4522321ced tools/ocaml/libs/xc: Backward compatible domid control at domain creation time
One can specify the domid to use when creating the domain, but this was
hardcoded to 0.

Keep the existing `domain_create` function, and make domid an optional
argument.  When not specified, default to 0.

Controlling the domid can be useful during testing or migration.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2021-01-22 18:01:34 +00:00
Edwin Török 2a47797d1f tools/oxenstored: Trim txhistory on xenbus reconnect
There is a global history, containing transactions from the past 0.05s, which
get trimmed whenever any transaction commits or aborts.  Destroying a domain
will cause xenopsd to perform some transactions deleting the tree, so that is
fine.  But I think that a domain can abuse the xenbus reconnect facility to
cause a large history to be recorded - provided that noone does any
transactions on the system inbetween, which may be difficult to achieve given
squeezed's constant pinging.

The theoretical situation is like this:
- a domain starts a transaction, creates as large a tree as it can, commits
  it. Then repeatedly:
    - start a transaction, do nothing with it, start a transaction, delete
      part of the large tree, write some new unique data there, don't commit
    - cause a xenbus reconnect (I think this can be done by writing something
      to the ring). This causes all transactions/watches for the connection to
      be cleared, but NOT the history, there were no commits, so nobody
      trimmed the history, i.e. it the history can contain transactions from
      more than just 0.05s
    - loop back and start more transactions, you can keep this up indefinitely
      without hitting quotas

Now there is a periodic History.trim running every 0.05s, so I don't think you
can do much damage with it.  But lets be safe an trim the transaction history
anyway on reconnect.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2021-01-22 18:01:34 +00:00
Edwin Török 42f0581a91 tools/oxenstored: Implement live update for socket connections
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Reviewed-by: Pau Ruiz Safont <pau.safont@citrix.com>
Reviewed-by: Christian Lindig <christian.lindig@citrix.com>
2021-01-22 18:01:33 +00:00
Edwin Török 00c48f57ab tools/oxenstored: Start live update process
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Reviewed-by: Pau Ruiz Safont <pau.safont@citrix.com>
Reviewed-by: Christian Lindig <christian.lindig@citrix.com>
2021-01-22 16:50:18 +00:00
Edwin Török 401c67e9bc tools/oxenstored: Add cooperative live-update command
See docs/misc/xenstore.txt for documentation on live-update command.  Validate
that the binary exists and that the cmdline is valid, to prevent typos from
taking down xenstore; if live-update fails there is no way back due to the use
of exec().

Live update only proceeds if there are no active transactions, and no
unprocessed input or unflushed output.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Reviewed-by: Pau Ruiz Safont <pau.safont@citrix.com>
Reviewed-by: Christian Lindig <christian.lindig@citrix.com>
2021-01-22 16:49:21 +00:00
Edwin Török 4387b4c771 tools/oxenstored: Automatically resume when possible
When a `db` file exists use it to resume oxenstored.  It will contains a
xenstore tree, domain reconnection info, and watches.

It is currently missing data about all active socket connections, so a
toolstack should ideally be stopped and restarted too.

Tell systemd about oxenstored's PID and allow it to restart on success.

This should make updating oxenstored as easy as: `systemctl stop -s SIGTERM
xenstored` on a suitable xenstored version.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Reviewed-by: Pau Ruiz Safont <pau.safont@citrix.com>
Reviewed-by: Christian Lindig <christian.lindig@citrix.com>
2021-01-22 15:22:42 +00:00
Edwin Török e663158bca tools/oxenstored: Only quit on SIGTERM when a reload is possible
Currently when oxenstored receives SIGTERM it dumps its state and quits.  It
is possible to then restart it if --restart is given, however that is not
always safe:

* Domains could have active transactions, and after a restart they would
  either reuse transaction IDs of already open transactions, or get an error
  back that the transaction doesn't exist

* There could be pending data to send to a VM still in oxenstored's
  queue which would be lost

* There could be pending input to be processed from a VM in oxenstored's
  queue which would be lost

Prevent shutting down oxenstored via SIGTERM in the above situations.  Also
ignore domains marked as bad because oxenstored would never talk to them
again.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Reviewed-by: Pau Ruiz Safont <pau.safont@citrix.com>
Reviewed-by: Christian Lindig <christian.lindig@citrix.com>
2021-01-22 14:55:10 +00:00
Edwin Török 5e31789634 tools/ocaml/libs/xb: Do not crash after xenbus is unmapped
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>
2021-01-22 14:40:29 +00:00
Edwin Török f437314e86 Makefile: Add build-tools-oxenstored
As a convenience so that oxenstored patches can be compile-tested
using upstream's build-system before submitting upstream.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
2021-01-22 14:39:48 +00:00
Manuel Bouyer cf2a566060 ocaml/libs/eventchn: drop unneeded evtchn.h
On NetBSD xen/sys/evtchn.h is not available any more. Just remove it as it's
not needed.

Signed-off-by: Manuel Bouyer <bouyer@netbsd.org>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2021-01-18 17:55:27 +00:00
Paul Durrant c00da82355 libxl: add libxl_device_pci_assignable_list_free()...
... to be used by callers of libxl_device_pci_assignable_list().

Currently there is no API for callers of libxl_device_pci_assignable_list()
to free the list. The xl function pciassignable_list() calls
libxl_device_pci_dispose() on each element of the returned list, but
libxl_pci_assignable() in libxl_pci.c does not. Neither does the implementation
of libxl_device_pci_assignable_list() call libxl_device_pci_init().

This patch adds the new API function, makes sure it is used everywhere and
also modifies libxl_device_pci_assignable_list() to initialize list
entries rather than just zeroing them.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
Acked-by: Wei Liu <wl@xen.org>
2020-12-15 16:24:23 +00:00
Edwin Török 2a3f8d12f0 tools/ocaml/xenstored: only Dom0 can change node owner
Otherwise we can give quota away to another domain, either causing it to run
out of quota, or in case of Dom0 use unbounded amounts of memory and bypass
the quota system entirely.

This was fixed in the C version of xenstored in 2006 (c/s db34d2aaa5,
predating the XSA process by 5 years).

It was also fixed in the mirage version of xenstore in 2012, with a unit test
demonstrating the vulnerability:

  https://github.com/mirage/ocaml-xenstore/commit/6b91f3ac46b885d0530a51d57a9b3a57d64923a7
  https://github.com/mirage/ocaml-xenstore/commit/22ee5417c90b8fda905c38de0d534506152eace6

but possibly without realising that the vulnerability still affected the
in-tree oxenstored (added c/s f44af66041 in 2010).

This is XSA-352.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-12-15 13:37:33 +01:00
Edwin Török 491a077ed4 tools/ocaml/xenstored: delete watch from trie too when resetting watches
c/s f8c72b5261 "oxenstored: implement XS_RESET_WATCHES" from Xen 4.6
introduced reset watches support in oxenstored by mirroring the change
in cxenstored.

However the OCaml version has some additional data structures to
optimize watch firing, and just resetting the watches in one of the data
structures creates a security bug where a malicious guest kernel can
exceed its watch quota, driving oxenstored into OOM:
 * create watches
 * reset watches (this still keeps the watches lingering in another data
   structure, using memory)
 * create some more watches
 * loop until oxenstored dies

The guest kernel doesn't necessarily have to be malicious to trigger
this:
 * if control/platform-feature-xs_reset_watches is set
 * the guest kexecs (e.g. because it crashes)
 * on boot more watches are set up
 * this will slowly "leak" memory for watches in oxenstored, driving it
   towards OOM.

This is XSA-330.

Fixes: f8c72b5261 ("oxenstored: implement XS_RESET_WATCHES")
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-12-15 13:37:14 +01:00
Edwin Török bf0703992c tools/ocaml/xenstored: Fix path length validation
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>
2020-12-15 13:36:39 +01:00
Edwin Török c46eff9212 tools/ocaml/xenstored: clean up permissions for dead domains
domain ids are prone to wrapping (15-bits), and with sufficient number
of VMs in a reboot loop it is possible to trigger it.  Xenstore entries
may linger after a domain dies, until a toolstack cleans it up. During
this time there is a window where a wrapped domid could access these
xenstore keys (that belonged to another VM).

To prevent this do a cleanup when a domain dies:
 * walk the entire xenstore tree and update permissions for all nodes
   * if the dead domain had an ACL entry: remove it
   * if the dead domain was the owner: change the owner to Dom0

This is done without quota checks or a transaction. Quota checks would
be a no-op (either the domain is dead, or it is Dom0 where they are not
enforced).  Transactions are not needed, because this is all done
atomically by oxenstored's single thread.

The xenstore entries owned by the dead domain are not deleted, because
that could confuse a toolstack / backends that are still bound to it
(or generate unexpected watch events). It is the responsibility of a
toolstack to remove the xenstore entries themselves.

This is part of XSA-322.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2020-12-15 13:36:04 +01:00
Edwin Török 34f008318d tools/ocaml/xenstored: add xenstored.conf flag to turn off watch permission checks
There are flags to turn off quotas and the permission system, so add one
that turns off the newly introduced watch permission checks as well.

This is part of XSA-115.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-12-15 13:35:19 +01:00
Edwin Török ff3d2dff9b tools/ocaml/xenstored: avoid watch events for nodes without access
Today watch events are sent regardless of the access rights of the
node the event is sent for. This enables any guest to e.g. setup a
watch for "/" in order to have a detailed record of all Xenstore
modifications.

Modify that by sending only watch events for nodes that the watcher
has a chance to see otherwise (either via direct reads or by querying
the children of a node). This includes cases where the visibility of
a node for a watcher is changing (permissions being removed).

Permissions for nodes are looked up either in the old (pre
transaction/command) or current trees (post transaction).  If
permissions are changed multiple times in a transaction only the final
version is checked, because considering a transaction atomic the
individual permission changes would not be noticable to an outside
observer.

Two trees are only needed for set_perms: here we can either notice the
node disappearing (if we loose permission), appearing
(if we gain permission), or changing (if we preserve permission).

RM needs to only look at the old tree: in the new tree the node would be
gone, or could have different permissions if it was recreated (the
recreation would get its own watch fired).

Inside a tree we lookup the watch path's parent, and then the watch path
child itself.  This gets us 4 sets of permissions in worst case, and if
either of these allows a watch, then we permit it to fire.  The
permission lookups are done without logging the failures, otherwise we'd
get confusing errors about permission denied for some paths, but a watch
still firing. The actual result is logged in xenstored-access log:

  'w event ...' as usual if watch was fired
  'w notfired...' if the watch was not fired, together with path and
  permission set to help in troubleshooting

Adding a watch bypasses permission checks and always fires the watch
once immediately. This is consistent with the specification, and no
information is gained (the watch is fired both if the path exists or
doesn't, and both if you have or don't have access, i.e. it reflects the
path a domain gave it back to that domain).

There are some semantic changes here:

  * Write+rm in a single transaction of the same path is unobservable
    now via watches: both before and after a transaction the path
    doesn't exist, thus both tree lookups come up with the empty
    permission set, and noone, not even Dom0 can see this. This is
    consistent with transaction atomicity though.
  * Similar to above if we temporarily grant and then revoke permission
    on a path any watches fired inbetween are ignored as well
  * There is a new log event (w notfired) which shows the permission set
    of the path, and the path.
  * Watches on paths that a domain doesn't have access to are now not
    seen, which is the purpose of the security fix.

This is part of XSA-115.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-12-15 13:35:16 +01:00
Edwin Török cf4116c38c tools/ocaml/xenstored: introduce permissions for special watches
The special watches "@introduceDomain" and "@releaseDomain" should be
allowed for privileged callers only, as they allow to gain information
about presence of other guests on the host. So send watch events for
those watches via privileged connections only.

Start to address this by treating the special watches as regular nodes
in the tree, which gives them normal semantics for permissions.  A later
change will restrict the handling, so that they can't be listed, etc.

This is part of XSA-115.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-12-15 13:35:13 +01:00
Edwin Török 0911dfad0f tools/ocaml/xenstored: unify watch firing
This will make it easier insert additional checks in a follow-up patch.
All watches are now fired from a single function.

This is part of XSA-115.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-12-15 13:35:11 +01:00
Edwin Török 67bfd6cd4b tools/ocaml/xenstored: check privilege for XS_IS_DOMAIN_INTRODUCED
The Xenstore command XS_IS_DOMAIN_INTRODUCED should be possible for privileged
domains only (the only user in the tree is the xenpaging daemon).

This is part of XSA-115.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-12-15 13:35:08 +01:00
Edwin Török f40d933da4 tools/ocaml/xenstored: ignore transaction id for [un]watch
Instead of ignoring the transaction id for XS_WATCH and XS_UNWATCH
commands as it is documented in docs/misc/xenstore.txt, it is tested
for validity today.

Really ignore the transaction id for XS_WATCH and XS_UNWATCH.

This is part of XSA-115.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-12-15 13:35:06 +01:00
Edwin Török feeafa007b tools/ocaml/xenstored: do permission checks on xenstore root
This was lacking in a disappointing number of places.

The xenstore root node is treated differently from all other nodes, because it
doesn't have a parent, and mutation requires changing the parent.

Unfortunately this lead to open-coding the special case for root into every
single xenstore operation, and out of all the xenstore operations only read
did a permission check when handling the root node.

This means that an unprivileged guest can:

 * xenstore-chmod / to its liking and subsequently write new arbitrary nodes
   there (subject to quota)
 * xenstore-rm -r / deletes almost the entire xenstore tree (xenopsd quickly
   refills some, but you are left with a broken system)
 * DIRECTORY on / lists all children when called through python
   bindings (xenstore-ls stops at /local because it tries to list recursively)
 * get-perms on / works too, but that is just a minor information leak

Add the missing permission checks, but this should really be refactored to do
the root handling and permission checks on the node only once from a single
function, instead of getting it wrong nearly everywhere.

This is XSA-353.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2020-12-15 13:33:43 +01:00
Juergen Gross 4664034cdc tools/libs: move official headers to common directory
Instead of each library having an own include directory move the
official headers to tools/include instead. This will drop the need to
link those headers to tools/include and there is no need any longer
to have library-specific include paths when building Xen.

While at it remove setting of the unused variable
PKG_CONFIG_CFLAGS_LOCAL in libs/*/Makefile.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Tested-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Ian Jackson <iwj@xenproject.org>
2020-10-23 15:50:51 +01:00
Edwin Török a06d3feea3 tools/ocaml/xenstored: drop the creation of the RO socket
The readonly flag was propagated but ignored, so this was essentially
equivalent to a RW socket.

C xenstored is dropping the RO socket too, so drop it from oxenstored too.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Acked-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
2020-10-08 12:54:00 +00:00
Andrew Cooper dbe399b174 xen/domctl: Introduce and use XEN_DOMCTL_CDF_nested_virt
Like other major areas of functionality, nested virt (or not) needs to be
known at domain creation time for sensible CPUID handling, and wants to be
known this early for sensible infrastructure handling in Xen.

Introduce XEN_DOMCTL_CDF_nested_virt and modify libxl to set it appropriately
when creating domains.  There is no need to adjust the ARM logic to reject the
use of this new flag.

No functional change yet.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wl@xen.org>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2020-10-06 12:28:37 +01:00
Juergen Gross 41aea82de2 tools/libxl: move libxenlight to tools/libs/light
Carve out all libxenlight related sources and move them to
tools/libs/light in order to use the generic library build environment.

The closely related sources for libxl-save-helper and the libxl test
environment are being moved, too.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
2020-10-01 13:26:09 +02:00
Juergen Gross af6c78d9dc tools: move libxenctrl below tools/libs
Today tools/libxc needs to be built after tools/libs as libxenctrl is
depending on some libraries in tools/libs. This in turn blocks moving
other libraries depending on libxenctrl below tools/libs.

So carve out libxenctrl from tools/libxc and move it into
tools/libs/ctrl.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org> (stubdom parts)
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> (python parts)
Acked-by: Wei Liu <wl@xen.org>
2020-09-09 10:57:25 +00:00
Edwin Török 80a868f0f6 tools/ocaml/xenstored: drop select based socket watching
Poll has been the default since 2014, I think we can safely say by now
that poll() works and we don't need to fall back to select().

This will allow fixing up the way we call poll to be more efficient
(and pave the way for introducing epoll support):
currently poll wraps the select API, which is inefficient.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2020-08-18 17:34:50 +01:00
Edwin Török ba02a2cd1c tools/ocaml/libs/xc: Fix ambiguous documentation comment
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2020-08-18 17:34:50 +01:00
Elliott Mitchell f3885e8c3c tools/ocaml: Default to useful build output
While hiding details of build output looks pretty to some, defaulting to
doing so deviates from the rest of Xen.  Switch the OCAML tools to match
everything else.

Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2020-07-21 14:49:36 +00:00
Edwin Török a6ed77f1e0 oxenstored: fix ABI breakage introduced in Xen 4.9.0
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>
2020-07-21 12:31:45 +00:00
Julien Grall 59b087e395 tools/ocaml: Fix stubs build when OCaml has been compiled with -safe-string
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>
2020-04-20 15:03:13 +01:00
Julien Grall 78686437e9 tools/ocaml: libxb: Avoid to use String_val() when value is bytes
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>
2020-04-20 15:03:07 +01:00
Julien Grall d92ba1aa7c tools/ocaml: libxb: Harden stub_header_of_string()
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>
2020-04-20 15:03:03 +01:00
Julien Grall 3bf69699ca tools/ocaml: libxc: Check error return in stub_xc_vcpu_context_get()
xc_vcpu_getcontext() may fail to retrieve the vcpu context. Rather than
ignoring the return value, check it and throw an error if needed.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2020-04-20 15:02:17 +01:00
Andrew Cooper 270ff9a835 tools/ocaml: Drop cpuid helpers
These have no callers, and the underlying infrastructure is about to be
rewritten completely.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2020-02-10 13:09:54 +00:00
Petre Pircalabu 368375d736 tools/ocaml: Fix build error with Arch Linux
gcc (GCC) 9.2.0 complains:

xentoollog_stubs.c: In function ‘stub_xtl_ocaml_vmessage’:
xentoollog_stubs.c:93:16: error: initialization discards ‘const’ qualifier from pointer target type [-Werror=discarded-qualifiers]
   93 |  value *func = caml_named_value(xtl->vmessage_cb) ;
      |                ^~~~~~~~~~~~~~~~

This patch constifies the pointer returned by caml_named_value in order
to the accommodate newer versions of OCaml.
In OCaml >= 4.09 the return value pointer of caml_named_value is
declared const.

https://github.com/ocaml/ocaml/commit/4f03a1467d29cf587df5a191830f1525506ee0e3

Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
[ wei: remove spaces before semicolons ]
Signed-off-by: Wei Liu <wl@xen.org>
2019-10-29 12:54:10 +00:00
Roger Pau Monne b183e180bc libxl: wait for the ack when issuing power control requests
Currently only suspend power control requests wait for an ack from the
domain, while power off or reboot requests simply write the command to
xenstore and exit.

Introduce a 1 minute wait for the domain to acknowledge the request, or
else return an error. The suspend code is slightly modified to use the
new infrastructure added, but shouldn't have any functional change.

Fix the ocaml bindings and also provide a backwards compatible
interface for the reboot and poweroff libxl API functions.

Reported-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
[ wei: change ret to rc to fix build ]
Signed-off-by: Wei Liu <wl@xen.org>
2019-10-02 14:43:19 +01:00
Paul Durrant babde47a3f introduce a 'passthrough' configuration option to xl.cfg...
...and hence the ability to disable IOMMU mappings, and control EPT
sharing.

This patch introduces a new 'libxl_passthrough' enumeration into
libxl_domain_create_info. The value will be set by xl either when it parses
a new 'passthrough' option in xl.cfg, or implicitly if there is passthrough
hardware specified for the domain.

If the value of the passthrough configuration option is 'disabled' then
the XEN_DOMCTL_CDF_iommu flag will be clear in the xen_domctl_createdomain
flags, thus allowing the toolstack to control whether the domain gets
IOMMU mappings or not (where previously they were globally set).

If the value of the passthrough configuration option is 'sync_pt' then
a new 'iommu_opts' field in xen_domctl_createdomain will be set with the
value XEN_DOMCTL_IOMMU_no_sharept. This will override the global default
set in iommu_hap_pt_share, thus allowing the toolstack to control whether
EPT sharing is used for the domain.

If the value of passthrough is 'enabled' then xl will choose an appropriate
default according to the type of domain and hardware support.

NOTE: The 'iommu_memkb' overhead in libxl_domain_build_info will now only
      be set if passthrough is 'sync_pt' (or xl has chosen this mode as
      a default).

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2019-09-25 16:14:55 +02:00
Ian Jackson 8d8db5a4ad tools/ocaml: abi check: Cope with consecutive relevant enums
If the end of one enum is the `type' line for the next enum, we would
not notice it.

Fix this by reordering the code, and getting rid of the else: now if
the "we are within an enum" branch decides that it's the end of the
enum, it unsets $ei and we then immediately process the line as a "not
within an enum" line - ie as the start of the next one.

Reported-by: Paul Durrant <paul.durrant@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2019-09-25 16:14:21 +02:00
Anthony PERARD e17381916e tools/ocaml: Build fix following libxl API changes
The following libxl API became asynchronous and gained an additional
`ao_how' parameter:
    libxl_domain_pause()
    libxl_domain_unpause()
    libxl_send_trigger()

Adapt the ocaml binding.

Build tested only.

Fixes: edaa631ddc
Fixes: 95627b87c3
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-09-23 11:14:41 +01:00