Commit Graph
15489 Commits
Author SHA1 Message Date
Juergen Gross a07cc0ec60 tools/xenstore: use treewalk for check_store()
Instead of doing an open tree walk using call recursion, use
walk_node_tree() when checking the store for inconsistencies.

This will reduce code size and avoid many nesting levels of function
calls which could potentially exhaust the stack.

This is part of XSA-418 / CVE-2022-42321.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 70f719f52a tools/xenstore: simplify check_store()
check_store() is using a hash table for storing all node names it has
found via walking the tree. Additionally it using another hash table
for all children of a node to detect duplicate child names.

Simplify that by dropping the second hash table as the first one is
already holding all the needed information.

This is part of XSA-418 / CVE-2022-42321.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 0d7c5d19bc tools/xenstore: add generic treewalk function
Add a generic function to walk the complete node tree. It will start
at "/" and descend recursively into each child, calling a function
specified by the caller. Depending on the return value of the user
specified function the walk will be aborted, continued, or the current
child will be skipped by not descending into its children.

This is part of XSA-418 / CVE-2022-42321.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 0c00c51f3b tools/xenstore: don't let remove_child_entry() call corrupt()
In case of write_node() returning an error, remove_child_entry() will
call corrupt() today. This could result in an endless recursion, as
remove_child_entry() is called by corrupt(), too:

corrupt()
  check_store()
    check_store_()
      remove_child_entry()

Fix that by letting remove_child_entry() return an error instead and
let the caller decide what to do.

This is part of XSA-418 / CVE-2022-42321.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross da8ee25d02 tools/xenstore: remove recursion from construct_node()
In order to reduce stack usage due to recursion, switch
construct_node() to use a loop instead.

This is part of XSA-418 / CVE-2022-42321.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross ab12821822 tools/xenstore: fix checking node permissions
Today chk_domain_generation() is being used to check whether a node
permission entry is still valid or whether it is referring to a domain
no longer existing. This is done by comparing the node's and the
domain's generation count.

In case no struct domain is existing for a checked domain, but the
domain itself is valid, chk_domain_generation() assumes it is being
called due to the first node created for a new domain and it will
return success.

This might be wrong in case the checked permission is related to an
old domain, which has just been replaced with a new domain using the
same domid.

Fix that by letting chk_domain_generation() fail in case a struct
domain isn't found. In order to cover the case of the first node for
a new domain try to allocate the needed struct domain explicitly when
processing the related SET_PERMS command. In case a referenced domain
isn't existing, flag the related permission to be ignored right away.

This is XSA-417 / CVE-2022-42320.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 2a587de219 tools/xenstore: don't use conn->in as context for temporary allocations
Using the struct buffered data pointer of the current processed request
for temporary data allocations has a major drawback: the used area (and
with that the temporary data) is freed only after the response of the
request has been written to the ring page or has been read via the
socket. This can happen much later in case a guest isn't reading its
responses fast enough.

As the temporary data can be safely freed after creating the response,
add a temporary context for that purpose and use that for allocating
the temporary memory, as it was already the case before commit
cc06124648 ("xenstore: add small default data buffer to internal
struct").

Some sub-functions need to gain the "const" attribute for the talloc
context.

This is XSA-416 / CVE-2022-42319.

Fixes: cc06124648 ("xenstore: add small default data buffer to internal struct")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Edwin Török 9284ae0c40 tools/ocaml: Limit maximum in-flight requests / outstanding replies
Introduce a limit on the number of outstanding reply packets in the xenbus
queue.  This limits the number of in-flight requests: when the output queue is
full we'll stop processing inputs until the output queue has room again.

To avoid a busy loop on the Unix socket we only add it to the watched input
file descriptor set if we'd be able to call `input` on it.  Even though Dom0
is trusted and exempt from quotas a flood of events might cause a backlog
where events are produced faster than daemons in Dom0 can consume them, which
could lead to an unbounded queue size and OOM.

Therefore the xenbus queue limit must apply to all connections, Dom0 is not
exempt from it, although if everything works correctly it will eventually
catch up.

This prevents a malicious guest from sending more commands while it has
outstanding watch events or command replies in its input ring.  However if it
can cause the generation of watch events by other means (e.g. by Dom0, or
another cooperative guest) and stop reading its own ring then watch events
would've queued up without limit.

The xenstore protocol doesn't have a back-pressure mechanism, and doesn't
allow dropping watch events.  In fact, dropping watch events is known to break
some pieces of normal functionality.  This leaves little choice to safely
implement the xenstore protocol without exposing the xenstore daemon to
out-of-memory attacks.

Implement the fix as pipes with bounded buffers:
* Use a bounded buffer for watch events
* The watch structure will have a bounded receiving pipe of watch events
* The source will have an "overflow" pipe of pending watch events it couldn't
  deliver

Items are queued up on one end and are sent as far along the pipe as possible:

  source domain -> watch -> xenbus of target -> xenstore ring/socket of target

If the pipe is "full" at any point then back-pressure is applied and we prevent
more items from being queued up.  For the source domain this means that we'll
stop accepting new commands as long as its pipe buffer is not empty.

Before we try to enqueue an item we first check whether it is possible to send
it further down the pipe, by attempting to recursively flush the pipes. This
ensures that we retain the order of events as much as possible.

We might break causality of watch events if the target domain's queue is full
and we need to start using the watch's queue.  This is a breaking change in
the xenstore protocol, but only for domains which are not processing their
incoming ring as expected.

When a watch is deleted its entire pending queue is dropped (no code is needed
for that, because it is part of the 'watch' type).

There is a cache of watches that have pending events that we attempt to flush
at every cycle if possible.

Introduce 3 limits here:
* quota-maxwatchevents on watch event destination: when this is hit the
  source will not be allowed to queue up more watch events.
* quota-maxoustanding which is the number of responses not read from the ring:
  once exceeded, no more inputs are processed until all outstanding replies
  are consumed by the client.
* overflow queue on the watch event source: all watches that cannot be stored
  on destination are queued up here, a single command can trigger multiple
  watches (e.g. due to recursion).

The overflow queue currently doesn't have an upper bound, it is difficult to
accurately calculate one as it depends on whether you are Dom0 and how many
watches each path has registered and how many watch events you can trigger
with a single command (e.g. a commit).  However these events were already
using memory, this just moves them elsewhere, and as long as we correctly
block a domain it shouldn't result in unbounded memory usage.

Note that Dom0 is not excluded from these checks, it is important that Dom0 is
especially not excluded when it is the source, since there are many ways in
which a guest could trigger Dom0 to send it watch events.

This should protect against malicious frontends as long as the backend follows
the PV xenstore protocol and only exposes paths needed by the frontend, and
changes those paths at most once as a reaction to guest events, or protocol
state.

The queue limits are per watch, and per domain-pair, so even if one
communication channel would be "blocked", others would keep working, and the
domain itself won't get blocked as long as it doesn't overflow the queue of
watch events.

Similarly a malicious backend could cause the frontend to get blocked, but
this watch queue protects the frontend as well as long as it follows the PV
protocol.  (Although note that protection against malicious backends is only a
best effort at the moment)

This is part of XSA-326 / CVE-2022-42318.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-11-01 13:05:44 +00:00
Edwin Török 19171fb5d8 tools/ocaml/xb: Add BoundedQueue
Ensures we cannot store more than [capacity] elements in a [Queue].  Replacing
all Queue with this module will then ensure at compile time that all Queues
are correctly bound checked.

Each element in the queue has a class with its own limits.  This, in a
subsequent change, will ensure that command responses can proceed during a
flood of watch events.

No functional change.

This is part of XSA-326.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-11-01 13:05:44 +00:00
Edwin Török c0a86a4627 tools/ocaml: Change Xb.input to return Packet.t option
The queue here would only ever hold at most one element.  This will simplify
follow-up patches.

This is part of XSA-326.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-11-01 13:05:44 +00:00
Edwin Török 4a8bacff20 tools/ocaml: GC parameter tuning
By default the OCaml garbage collector would return memory to the OS only
after unused memory is 5x live memory.  Tweak this to 120% instead, which
would match the major GC speed.

This is part of XSA-326.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-11-01 13:05:44 +00:00
Edwin Török 329f4d1a65 tools/ocaml/xenstored: Check for maxrequests before performing operations
Previously we'd perform the operation, record the updated tree in the
transaction record, then try to insert a watchop path and the reply packet.

If we exceeded max requests we would've returned EQUOTA, but still:
* have performed the operation on the transaction's tree
* have recorded the watchop, making this queue effectively unbounded

It is better if we check whether we'd have room to store the operation before
performing the transaction, and raise EQUOTA there.  Then the transaction
record won't grow.

This is part of XSA-326 / CVE-2022-42317.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-11-01 13:05:44 +00:00
Edwin Török 84734955d4 tools/ocaml/xenstored: Synchronise defaults with oxenstore.conf.in
We currently have 2 different set of defaults in upstream Xen git tree:
* defined in the source code, only used if there is no config file
* defined in the oxenstored.conf.in upstream Xen

An oxenstored.conf file is not mandatory, and if missing, maxrequests in
particular has an unsafe default.

Resync the defaults from oxenstored.conf.in into the source code.

This is part of XSA-326 / CVE-2022-42316.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 9c484bef83 tools/xenstore: add control command for setting and showing quota
Add a xenstore-control command "quota" to:
- show current quota settings
- change quota settings
- show current quota related values of a domain

Note that in the case the new quota is lower than existing one,
Xenstored may continue to handle requests from a domain exceeding the
new limit (depends on which one has been broken) and the amount of
resource used will not change. However the domain will not be able to
create more resource (associated to the quota) until it is back to below
the limit.

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 1da16d5990 tools/xenstore: add exports for quota variables
Some quota variables are not exported via header files.

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 00e9e32d02 tools/xenstore: add memory accounting for nodes
Add the memory accounting for Xenstore nodes. In order to make this
not too complicated allow for some sloppiness when writing nodes. Any
hard quota violation will result in no further requests to be accepted.

This is part of XSA-326 / CVE-2022-42315.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 7f9978a2cc tools/xenstore: add memory accounting for watches
Add the memory accounting for registered watches.

When a socket connection is destroyed, the associated watches are
removed, too. In order to keep memory accounting correct the watches
must be removed explicitly via a call of conn_delete_all_watches() from
destroy_conn().

This is part of XSA-326 / CVE-2022-42315.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross f6d0013364 tools/xenstore: add memory accounting for responses
Add the memory accounting for queued responses.

In case adding a watch event for a guest is causing the hard memory
quota of that guest to be violated, the event is dropped. This will
ensure that it is impossible to drive another guest past its memory
quota by generating insane amounts of events for that guest. This is
especially important for protecting driver domains from that attack
vector.

This is part of XSA-326 / CVE-2022-42315.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 0d4a8ec7a9 tools/xenstore: add infrastructure to keep track of per domain memory usage
The amount of memory a domain can consume in Xenstore is limited by
various quota today, but even with sane quota a domain can still
consume rather large memory quantities.

Add the infrastructure for keeping track of the amount of memory a
domain is consuming in Xenstore. Note that this is only the memory a
domain has direct control over, so any internal administration data
needed by Xenstore only is not being accounted for.

There are two quotas defined: a soft quota which will result in a
warning issued via syslog() when it is exceeded, and a hard quota
resulting in a stop of accepting further requests or watch events as
long as the hard quota would be violated by accepting those.

Setting any of those quotas to 0 will disable it.

As default values use 2MB per domain for the soft limit (this basically
covers the allowed case to create 1000 nodes needing 2kB each), and
2.5MB for the hard limit.

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 60e2f6020d tools/xenstore: move the call of setup_structure() to dom0 introduction
Setting up the basic structure when introducing dom0 has the advantage
to be able to add proper node memory accounting for the added nodes
later.

This makes it possible to do proper node accounting, too.

An additional requirement to make that work fine is to correct the
owner of the created nodes to be dom0_domid instead of domid 0.

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 268369d8e3 tools/xenstore: limit max number of nodes accessed in a transaction
Today a guest is free to access as many nodes in a single transaction
as it wants. This can lead to unbounded memory consumption in Xenstore
as there is the need to keep track of all nodes having been accessed
during a transaction.

In oxenstored the number of requests in a transaction is being limited
via a quota maxrequests (default is 1024). As multiple accesses of a
node are not problematic in C Xenstore, limit the number of accessed
nodes.

In order to let read_node() detect a quota error in case too many nodes
are being accessed, check the return value of access_node() and return
NULL in case an error has been seen. Introduce __must_check and add it
to the access_node() prototype.

This is part of XSA-326 / CVE-2022-42314.

Suggested-by: Julien Grall <julien@xen.org>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross dbef1f7482 tools/xenstore: simplify and fix per domain node accounting
The accounting of nodes can be simplified now that each connection
holds the associated domid.

Fix the node accounting to cover nodes created for a domain before it
has been introduced. This requires to react properly to an allocation
failure inside domain_entry_inc() by returning an error code.

Especially in error paths the node accounting has to be fixed in some
cases.

This is part of XSA-326 / CVE-2022-42313.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 3047df38e1 tools/xenstore: fix connection->id usage
Don't use conn->id for privilege checks, but domain_is_unprivileged().

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross b5c0bdb96d tools/xenstore: don't buffer multiple identical watch events
A guest not reading its Xenstore response buffer fast enough might
pile up lots of Xenstore watch events buffered. Reduce the generated
load by dropping new events which already have an identical copy
pending.

The special events "@..." are excluded from that handling as there are
known use cases where the handler is relying on each event to be sent
individually.

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 36de433a27 tools/xenstore: limit outstanding requests
Add another quota for limiting the number of outstanding requests of a
guest. As the way to specify quotas on the command line is becoming
rather nasty, switch to a new scheme using [--quota|-Q] <what>=<val>
allowing to add more quotas in future easily.

Set the default value to 20 (basically a random value not seeming to
be too high or too low).

A request is said to be outstanding if any message generated by this
request (the direct response plus potential watch events) is not yet
completely stored into a ring buffer. The initial watch event sent as
a result of registering a watch is an exception.

Note that across a live update the relation to buffered watch events
for other domains is lost.

Use talloc_zero() for allocating the domain structure in order to have
all per-domain quota zeroed initially.

This is part of XSA-326 / CVE-2022-42312.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 5285dcb1a5 tools/xenstore: let unread watch events time out
A future modification will limit the number of outstanding requests
for a domain, where "outstanding" means that the response of the
request or any resulting watch event hasn't been consumed yet.

In order to avoid a malicious guest being capable to block other guests
by not reading watch events, add a timeout for watch events. In case a
watch event hasn't been consumed after this timeout, it is being
deleted. Set the default timeout to 20 seconds (a random value being
not too high).

In order to support to specify other timeout values in future, use a
generic command line option for that purpose:

--timeout|-w watch-event=<seconds>

This is part of XSA-326 / CVE-2022-42311.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 3a96013a3e tools/xenstore: reduce number of watch events
When removing a watched node outside of a transaction, two watch events
are being produced instead of just a single one.

When finalizing a transaction watch events can be generated for each
node which is being modified, even if outside a transaction such
modifications might not have resulted in a watch event.

This happens e.g.:

- for nodes which are only modified due to added/removed child entries
- for nodes being removed or created implicitly (e.g. creation of a/b/c
  is implicitly creating a/b, resulting in watch events for a, a/b and
  a/b/c instead of a/b/c only)

Avoid these additional watch events, in order to reduce the needed
memory inside Xenstore for queueing them.

This is being achieved by adding event flags to struct accessed_node
specifying whether an event should be triggered, and whether it should
be an exact match of the modified path. Both flags can be set from
fire_watches() instead of implying them only.

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross ead062a68a tools/xenstore: add helpers to free struct buffered_data
Add two helpers for freeing struct buffered_data: free_buffered_data()
for freeing one instance and conn_free_buffered_data() for freeing all
instances for a connection.

This is avoiding duplicated code and will help later when more actions
are needed when freeing a struct buffered_data.

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Juergen Gross 9bfde319db tools/xenstore: split up send_reply()
Today send_reply() is used for both, normal request replies and watch
events.

Split it up into send_reply() and send_event(). This will be used to
add some event specific handling.

add_event() can be merged into send_event(), removing the need for an
intermediate memory allocation.

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2022-11-01 13:05:44 +00:00
Julien Grall 5d71766bd1 tools/xenstore: Fail a transaction if it is not possible to create a node
Commit f2bebf72c4 "xenstore: rework of transaction handling" moved
out from copying the entire database everytime a new transaction is
opened to track the list of nodes changed.

The content of all the nodes accessed during a transaction will be
temporarily stored in TDB using a different key.

The function create_node() may write/update multiple nodes if the child
doesn't exist. In case of a failure, the function will revert any
changes (this include any update to TDB). Unfortunately, the function
which reverts the changes (i.e. destroy_node()) will not use the correct
key to delete any update or even request the transaction to fail.

This means that if a client decide to go ahead with committing the
transaction, orphan nodes will be created because they were not linked
to an existing node (create_node() will write the nodes backwards).

Once some nodes have been partially updated in a transaction, it is not
easily possible to undo any changes. So rather than continuing and hit
weird issue while committing, it is much saner to fail the transaction.

This will have an impact on any client that decides to commit even if it
can't write a node. Although, it is not clear why a normal client would
want to do that...

Lastly, update destroy_node() to use the correct key for deleting the
node. Rather than recreating it (this will allocate memory and
therefore fail), stash the key in the structure node.

This is XSA-415 / CVE-2022-42310.

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
2022-11-01 13:05:44 +00:00
Julien Grall 1cd3cc7ea2 tools/xenstore: create_node: Don't defer work to undo any changes on failure
XSA-115 extended destroy_node() to update the node accounting for the
connection. The implementation is assuming the connection is the parent
of the node, however all the nodes are allocated using a separate context
(see process_message()). This will result to crash (or corrupt) xenstored
as the pointer is wrongly used.

In case of an error, any changes to the database or update to the
accounting will now be reverted in create_node() by calling directly
destroy_node(). This has the nice advantage to remove the loop to unset
the destructors in case of success.

Take the opportunity to free the nodes right now as they are not
going to be reachable (the function returns NULL) and are just wasting
resources.

This is XSA-414 / CVE-2022-42309.

Fixes: 0bfb2101f2 ("tools/xenstore: fix node accounting after failed node creation")
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
2022-11-01 13:05:44 +00:00
Anthony PERARD 4ff0811a2b libs/light: Fix build, fix missing _libxl_types_json.h
Make may not have copied "_libxl_types_json.h" into $(XEN_INCLUDE)
before starting to build the different objects.

Make sure that the generated headers are copied into $(XEN_INCLUDE)
before using them. This is achieved by telling make about which
headers are needed to use "libxl_internal.h" which use "libxl_json.h"
which uses "_libxl_types_json.h". "libxl_internal.h" also uses
"libxl.h" so add it to the list.

This also prevent `gcc` from using a potentially installed headers
from a previous version of Xen.

Reported-by: Per Bilse <per.bilse@citrix.com>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-25 13:36:40 +01:00
Edwin Török f838b95677 tools/ocaml/xenstored: fix live update exception
During live update we will load the /tool/xenstored path from the previous binary,
and then try to mkdir /tool again which will fail with EEXIST.
Check for existence of the path before creating it.

The write call to /tool/xenstored should not need any changes
(and we do want to overwrite any previous path, in case it changed).

Prior to 7110192b1d live update would work only if the binary path was
specified, and with 7110192b1d and this live update also works when
no binary path is specified in `xenstore-control live-update`.

Fixes: 7110192b1d ("tools/oxenstored: Fix Oxenstored Live Update")
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-21 10:28:12 +01:00
Peter Hoyes 0c06760be3 tools/xendomains: Restrict domid pattern in LIST_GREP
The xendomains script uses the output of `xl list -l` to collect the
id and name of each domain, which is used in the shutdown logic, amongst
other purposes.

The linked commit added a "domid" field to libxl_domain_create_info.
This causes the output of `xl list -l` to contain two "domid"s per
domain, which may not be equal. This in turn causes `xendomains stop` to
issue two shutdown commands per domain, one of which is to a duplicate
and/or invalid domid.

To work around this, make the LIST_GREP pattern more restrictive for
domid, so it only detects the domid at the top level and not the domid
inside c_info.

Fixes: 4a3a25678d ("libxl: allow creation of domains with a specified or random domid")
Signed-off-by: Peter Hoyes <Peter.Hoyes@arm.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-20 17:38:56 +01:00
Andrew Cooper 7110192b1d tools/oxenstored: Fix Oxenstored Live Update
tl;dr This hunk was part of the patch emailed to xen-devel, but was missing
from what ultimately got committed.

https://lore.kernel.org/xen-devel/4164cb728313c3b9fc38cf5e9ecb790ac93a9600.1610748224.git.edvin.torok@citrix.com/
is the patch in question, but was part of a series that had threading issues.
I have a vague recollection that I sourced the commits from a local branch,
which clearly wasn't as up-to-date as I had thought.

Either way, it's my fault/mistake, and this hunk should have been part of what
got comitted.

Fixes: 00c48f57ab ("tools/oxenstored: Start live update process")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-20 15:48:22 +01:00
Roger Pau Monné e9444d8742 test/vpci: enable by default
CONFIG_HAS_PCI is not defined for the tools build, and as a result the
vpci harness would never get build.  Fix this by building it
unconditionally, there's nothing arch specific in it.

Reported-by: Andrew Cooper <Andrew.Cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-20 16:37:29 +02:00
Roger Pau Monné 1cfccd4b07 test/vpci: fix vPCI test harness to provide pci_get_pdev()
Instead of pci_get_pdev_by_domain(), which is no longer present in the
hypervisor.

While there add parentheses around the define value.

Fixes: a37f9ea7a6 ('PCI: fold pci_get_pdev{,_by_domain}()')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-20 16:37:15 +02:00
Roger Pau Monné b71419530d test/vpci: add dummy cfcheck define
Some vpci functions got the cfcheck attribute added, but that's not
defined in the user-space test harness, so add a dummy define in order
for the harness to build.

Fixes: 4ed7d5525f ('xen/vpci: CFI hardening')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-20 16:36:48 +02:00
Anthony PERARD cc4747be8b tools: Workaround wrong use of tools/Rules.mk by qemu-trad
qemu-trad build system, when built from xen.git, will make use of
Rules.mk (setup via qemu-trad.git/xen-setup). This mean that changes
to Rules.mk will have an impact our ability to build qemu-trad.

Recent commit e4f5949c44 ("tools: Add -Werror by default to all
tools/") have added "-Werror" to the CFLAGS and qemu-trad start to use
it. But this fails and there's lots of warning that are now turned
into error.

We should teach qemu-trad and xen.git to not have to use Rules.mk when
building qemu-trad, but for now, avoid adding -Werror to CFLAGS when
building qemu-trad.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-17 14:57:34 +01:00
Anthony PERARD 5310a3aa50 tools: Rework linking options for ocaml binding libraries
Using a full path to the C libraries when preparing one of the ocaml
binding for those libraries make the binding unusable by external
project. The full path is somehow embedded and reused by the external
project when linking against the binding.

Instead, we will use the proper way to link a library, by using '-l'.
For in-tree build, we also need to provide the search directory via
'-L'.

(The search path -L are still be embedded, but at least that doesn't
prevent the ocaml binding from been used.)

Related-to: xen-project/xen#96
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-14 20:56:57 +01:00
Anthony PERARD 3f9d53af25 tools/golang/xenlight: Rework gengotypes.py and generation of *.gen.go
gengotypes.py creates both "types.gen.go" and "helpers.gen.go", but
make can start gengotypes.py twice. Rework the rules so that
gengotypes.py is executed only once.

Also, add the ability to provide a path to tell gengotypes.py where to
put the files. This doesn't matter yet but it will when for example
the script will be run from tools/ to generate the targets.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-14 20:56:57 +01:00
Anthony PERARD 68d19cfb90 libs/light: Rework generation of include/_libxl_*.h
Instead of moving the public "_libxl_*.h" headers, we make a copy to
the destination so that make doesn't try to remake the targets
"_libxl_*.h" in libs/light/ again.

A new .PRECIOUS target is added to tell make to not deletes the
intermediate targets generated by "gentypes.py".

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-14 20:56:57 +01:00
Anthony PERARD 9eb46d3f98 libs/light: Rework acpi table build targets
Currently, a rebuild of libxl will always rebuild "build.o". This is because
the target depends on "acpi" which never exist. So instead we will have
"build.o" have as prerequisites targets that are actually generated by "acpi",
that is $(DSDT_FILES-y).

While "dsdt_*.c" isn't really a dependency for "build.o", a side
effect of building that dsdt_*.c is to also generate the "ssdt_*.h"
that "build.o" needs, but I don't want to list all the headers needed
by "build.o" and duplicate the information available in
"libacpi/Makefile" at this time.

Also avoid duplicating the "acpi" target for Arm, and unique one for
both architecture. And move the "acpi" target to be with other targets
rather than in the middle of the source listing. For the same reason,
move the prerequisites listing for both $(DSDT_FILES-y) and "build.o".

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-14 20:56:57 +01:00
Anthony PERARD 6aabee32b5 tools/include: Rework Makefile
Rework "xen-xsm" rules to not have to change directory to run
mkflask.sh, and store mkflask.sh path in a var, and use a full path
for FLASK_H_DEPEND, and output directory is made relative.

Rename "all-y" target to a more descriptive "xen/lib/x86/all".

Removed the "dist" target which was the only one existing in tools/.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-14 20:56:57 +01:00
Anthony PERARD 13c05b9efa libs: Avoid exposing -Wl,--version-script to other built library
$(SHLIB_LDFLAGS) is used by more targets that the single targets that
except it (libxenfoo.so.X.Y). There is also some dynamic libraries in
stats/ that uses $(SHLIB_LDFLAGS) (even if those are never built), and
there's libxenlight_test.so which doesn't needs a version script.

Also, libxenlight_test.so might failed to build if the version script
doesn't exist yet.

For these reasons, avoid changing the generic $(SHLIB_LDFLAGS) flags,
and add the flag directly on the command line.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-14 20:56:57 +01:00
Anthony PERARD 4c1a3cca79 libs/light/gentypes.py: allow to generate headers in subdirectory
This doesn't matter yet but it will when for example the script will
be run from tools/ to generate files tools/libs/light/.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-14 20:56:57 +01:00
Anthony PERARD f3fae4184f tools/hotplug: Generate "hotplugpath.sh" with configure
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-14 20:56:57 +01:00
Anthony PERARD 40d96f0c7d tools: Remove -Werror everywhere else
The previous changeset, e4f5949c44 ("tools: Add -Werror by default to all
tools/"), added "-Werror" to CFLAGS in tools/Rules.mk.  Remove it from
everywhere else now it is duplicated.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-14 16:17:41 +01:00
Anthony PERARD e4f5949c44 tools: Add -Werror by default to all tools/
And provide an option to ./configure to disable it.

A follow-up patch will remove -Werror from every other Makefile in
tools/.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-14 16:16:54 +01:00
Anthony PERARD fcdb9cdb95 tools: Introduce $(xenlibs-ldflags, ) macro
This avoid the need to open-coding the list of flags needed to link
with an in-tree Xen library when using -lxen*.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Henry Wang <Henry.Wang@arm.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-14 16:16:54 +01:00