When finalizing a transaction, any error occurring after checking for
conflicts will result in the transaction being performed only
partially today. Additionally accounting data will not be updated at
the end of the transaction, which might result in further problems
later.
Avoid those problems by multiple modifications:
- free any transaction specific nodes which don't need to be committed
as they haven't been written during the transaction as soon as their
generation count has been verified, this will reduce the risk of
out-of-memory situations
- store the transaction specific node name in struct accessed_node in
order to avoid the need to allocate additional memory for it when
finalizing the transaction
- don't stop the transaction finalization when hitting an error
condition, but try to continue to handle all modified nodes
- in case of a detected error do the accounting update as needed and
call the data base checking only after that
- if writing a node in a transaction is failing (e.g. due to a failed
quota check), fail the transaction, as prior changes to struct
accessed_node can't easily be undone in that case
This is part of XSA-421 / CVE-2022-42326.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Tested-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 2dd823ca72)
In case a node has been created in a transaction and it is later
deleted in the same transaction, the transaction will be terminated
with an error.
As this error is encountered only when handling the deleted node at
transaction finalization, the transaction will have been performed
partially and without updating the accounting information. This will
enable a malicious guest to create arbitrary number of nodes.
This is part of XSA-421 / CVE-2022-42325.
Signed-off-by: Juergen Gross <jgross@suse.com>
Tested-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 13ac37f141)
The permission scheme of Xenstore nodes is not really covered by
docs/misc/xenstore.txt, other than referring to the Xen wiki.
Add a paragraph explaining the permissions of nodes, and especially
mentioning removal of nodes when a domain has been removed from
Xenstore.
This is part of XSA-419.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit d084d2c6df)
Having a file backed data base has the only advantage of being capable
to dump the contents of it while Xenstore is running, and potentially
using less swap space in case the data base can't be kept in memory.
It has the major disadvantage of a huge performance overhead: switching
to keep the data base in memory only speeds up live update of xenstored
with 120000 nodes from 20 minutes to 11 seconds. A complete tree walk
of this configuration will be reduced from 7 seconds to 280 msecs
(measured by "xenstore-control check").
So make the internal memory data base the default and enhance the
"--internal-db" command line parameter to take an optional parameter
allowing to switch the internal data base back to the file based one.
This is part of XSA-419.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit d174fefa90)
In case a domain is removed from Xenstore, remove all nodes owned by
it per default.
This tackles the problem that nodes might be created by a domain
outside its home path in Xenstore, leading to Xenstore hogging more
and more memory. Domain quota don't work in this case if the guest is
rebooting in between.
Since XSA-322 ownership of such stale nodes is transferred to dom0,
which is helping against unintended access, but not against OOM of
Xenstore.
As a fallback for weird cases add a Xenstore start parameter for
keeping today's way to handle stale nodes, adding the risk of Xenstore
hitting an OOM situation.
This is part of XSA-419 / CVE-2022-42322.
Fixes: 496306324d ("tools/xenstore: revoke access rights for removed domains")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 755d3f9deb)
Instead of doing an open tree walk using call recursion, use
walk_node_tree() when deleting a sub-tree of nodes.
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>
Acked-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit ea16962053)
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>
(cherry picked from commit a07cc0ec60)
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>
(cherry picked from commit 70f719f52a)
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>
(cherry picked from commit 0d7c5d19bc)
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>
(cherry picked from commit 0c00c51f3b)
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>
(cherry picked from commit da8ee25d02)
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>
(cherry picked from commit ab12821822)
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>
(cherry picked from commit 2a587de219)
Add a support statement for the scope of support regarding different
Xenstore variants. Especially oxenstored does not (yet) have security
support of untrusted driver domains, as those might drive oxenstored
out of memory by creating lots of watch events for the guests they are
servicing.
Add a statement regarding Live Update support of oxenstored.
This is part of XSA-326.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit c7bc20d8d1)
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>
(cherry picked from commit 9c484bef83)
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>
(cherry picked from commit 1da16d5990)
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>
(cherry picked from commit 00e9e32d02)
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>
(cherry picked from commit 7f9978a2cc)
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>
(cherry picked from commit f6d0013364)
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>
(cherry picked from commit 0d4a8ec7a9)
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>
(cherry picked from commit 60e2f6020d)
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>
(cherry picked from commit 268369d8e3)
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>
(cherry picked from commit dbef1f7482)
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>
(cherry picked from commit 3047df38e1)
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>
(cherry picked from commit b5c0bdb96d)
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>
(cherry picked from commit 36de433a27)
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>
(cherry picked from commit 5285dcb1a5)
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>
(cherry picked from commit 3a96013a3e)
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>
(cherry picked from commit ead062a68a)
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>
(cherry picked from commit 9bfde319db)
The size of the video memory of PVH guests should be set to 0 in case
no value has been specified.
Doing not so will leave it to be -1, resulting in an additional 1 kB
of RAM being advertised in the memory map (here the output of a PVH
Mini-OS boot with 16 MB of RAM assigned):
Memory map:
000000000000-0000010003ff: RAM
0000feff8000-0000feffffff: Reserved
0000fc008000-0000fc00803f: ACPI
0000fc000000-0000fc000fff: ACPI
0000fc001000-0000fc007fff: ACPI
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
master commit: 0a20a53df1
master date: 2021-12-09 16:26:29 +0000
With smt=0 during a suspend/resume cycle of the machine the threads
which have been parked before will briefly come up again. This can
result in problems e.g. with cpufreq driver being active as this will
call into get_cpu_idle_time() for a cpu without initialized scheduler
data.
Fix that by letting get_cpu_idle_time() deal with this case. Drop a
redundant check in exchange.
Fixes: 132cbe8f35 ("sched: fix get_cpu_idle_time() with core scheduling")
Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Dario Faggioli <dfaggioli@suse.com>
master commit: 5293470a77
master date: 2021-08-19 13:38:31 +0200
By setting a watch with a very large tag it is possible to trick
xenstored to send watch event messages exceeding the maximum allowed
payload size. This might in turn lead to a crash of xenstored as the
resulting error can cause dereferencing a NULL pointer in case there
is no active request being handled by the guest the watch event is
being sent to.
Fix that by just dropping such watch events. Additionally modify the
error handling to test the pointer to be not NULL before dereferencing
it.
This is XSA-324.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Access rights of Xenstore nodes are per domid. Unfortunately existing
granted access rights are not removed when a domain is being destroyed.
This means that a new domain created with the same domid will inherit
the access rights to Xenstore nodes from the previous domain(s) with
the same domid.
This can be avoided by adding a generation counter to each domain.
The generation counter of the domain is set to the global generation
counter when a domain structure is being allocated. When reading or
writing a node all permissions of domains which are younger than the
node itself are dropped. This is done by flagging the related entry
as invalid in order to avoid modifying permissions in a way the user
could detect.
A special case has to be considered: for a new domain the first
Xenstore entries are already written before the domain is officially
introduced in Xenstore. In order not to drop the permissions for the
new domain a domain struct is allocated even before introduction if
the hypervisor is aware of the domain. This requires adding another
bool "introduced" to struct domain in xenstored. In order to avoid
additional padding holes convert the shutdown flag to bool, too.
As verifying permissions has its price regarding runtime add a new
quota for limiting the number of permissions an unprivileged domain
can set for a node. The default for that new quota is 5.
This is part of XSA-322.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Acked-by: Julien Grall <julien@amazon.com>
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).
This is part of XSA-115.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Paul Durrant <paul@xen.org>
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.
In order to allow for disaggregated setups where e.g. driver domains
need to make use of those special watches add support for calling
"set permissions" for those special nodes, too.
This is part of XSA-115.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Paul Durrant <paul@xen.org>
There are several places in xenstored using a permission array and the
size of that array. Introduce a new struct node_perms containing both.
This is part of XSA-115.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Instead of firing all watches for removing a subtree in one go, do so
only when the related node is being removed.
The watches for the top-most node being removed include all watches
including that node, while watches for nodes below that are only fired
if they are matching exactly. This avoids firing any watch more than
once when removing a subtree.
This is part of XSA-115.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Today a Xenstore node is being removed by deleting it from the parent
first and then deleting itself and all its children. This results in
stale entries remaining in the data base in case e.g. a memory
allocation is failing during processing. This would result in the
rather strange behavior to be able to read a node (as its still in the
data base) while not being visible in the tree view of Xenstore.
Fix that by deleting the nodes from the leaf side instead of starting
at the root.
As fire_watches() is now called from _rm() the ctx parameter needs a
const attribute.
This is part of XSA-115.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Paul Durrant <paul@xen.org>
The Xenstore command XS_IS_DOMAIN_INTRODUCED should be possible for
privileged domains only (the only user in the tree is the xenpaging
daemon).
Instead of having the privilege test for each command introduce a
per-command flag for that purpose.
This is part of XSA-115.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Paul Durrant <paul@xen.org>
There is no path which allows to call check_event_node() without a
event name. So don't let the result depend on the name being NULL and
add an assert() covering that case.
Rename the function to check_special_event() to better match the
semantics.
This is part of XSA-115.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Paul Durrant <paul@xen.org>
When a node creation fails the number of nodes of the domain should be
the same as before the failed node creation. In case of failure when
trying to create a node requiring to create one or more intermediate
nodes as well (e.g. when /a/b/c/d is to be created, but /a/b isn't
existing yet) it might happen that the number of nodes of the creating
domain is not reset to the value it had before.
So move the quota accounting out of construct_node() and into the node
write loop in create_node() in order to be able to undo the accounting
in case of an error in the intermediate node destructor.
This is part of XSA-115.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Acked-by: Julien Grall <jgrall@amazon.com>
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: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Paul Durrant <paul@xen.org>
An unprivileged user of Xenstore is not allowed to write nodes with a
size exceeding a global quota, while privileged users like dom0 are
allowed to write such nodes. The size of a node is the needed space
to store all node specific data, this includes the names of all
children of the node.
When deleting a node its parent has to be modified by removing the
name of the to be deleted child from it.
This results in the strange situation that an unprivileged owner of a
node might not succeed in deleting that node in case its parent is
exceeding the quota of that unprivileged user (it might have been
written by dom0), as the user is not allowed to write the updated
parent node.
Fix that by not checking the quota when writing a node for the
purpose of removing a child's name only.
The same applies to transaction handling: a node being read during a
transaction is written to the transaction specific area and it should
not be tested for exceeding the quota, as it might not be owned by
the reader and presumably the original write would have failed if the
node is owned by the reader.
This is part of XSA-115.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Two cpus entering evtchn_fifo_set_pending() for the same event channel
can race in case the first one gets interrupted after setting
EVTCHN_FIFO_PENDING and when the other one manages to set
EVTCHN_FIFO_LINKED before the first one is testing that bit. This can
lead to evtchn_check_pollers() being called before the event is put
properly into the queue, resulting eventually in the guest not seeing
the event pending and thus blocking forever afterwards.
Note that commit 5f2df45ead ("xen/evtchn: rework per event channel
lock") made the race just more obvious, while the fifo event channel
implementation had this race forever since the introduction and use of
per-channel locks, when an unmask operation was running in parallel with
an event channel send operation.
Using a spinlock for the per event channel lock had turned out
problematic due to some paths needing to take the lock are called with
interrupts off, so the lock would need to disable interrupts, which in
turn broke some use cases related to vm events.
For avoiding this race the queue locking in evtchn_fifo_set_pending()
needs to be reworked to cover the test of EVTCHN_FIFO_PENDING,
EVTCHN_FIFO_MASKED and EVTCHN_FIFO_LINKED, too. Additionally when an
event channel needs to change queues both queues need to be locked
initially, in order to avoid having a window with no lock held at all.
Reported-by: Jan Beulich <jbeulich@suse.com>
Fixes: 5f2df45ead ("xen/evtchn: rework per event channel lock")
Fixes: de6acb78bf ("evtchn: use a per-event channel lock for sending events")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 71ac522909
master date: 2020-11-30 14:05:39 +0100
The queue for a fifo event is depending on the vcpu_id and the
priority of the event. When sending an event it might happen the
event needs to change queues and the old queue needs to be kept for
keeping the links between queue elements intact. For this purpose
the event channel contains last_priority and last_vcpu_id values
elements for being able to identify the old queue.
In order to avoid races always access last_priority and last_vcpu_id
with a single atomic operation avoiding any inconsistencies.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
master commit: 1277cb9dc5
master date: 2020-11-24 11:23:42 +0100
Currently the lock for a single event channel needs to be taken with
interrupts off, which causes deadlocks in some cases.
Rework the per event channel lock to be non-blocking for the case of
sending an event and removing the need for disabling interrupts for
taking the lock.
The lock is needed for avoiding races between event channel state
changes (creation, closing, binding) against normal operations (set
pending, [un]masking, priority changes).
Use a rwlock, but with some restrictions:
- Changing the state of an event channel (creation, closing, binding)
needs to use write_lock(), with ASSERT()ing that the lock is taken as
writer only when the state of the event channel is either before or
after the locked region appropriate (either free or unbound).
- Sending an event needs to use read_trylock() mostly, in case of not
obtaining the lock the operation is omitted. This is needed as
sending an event can happen with interrupts off (at least in some
cases).
- Dumping the event channel state for debug purposes is using
read_trylock(), too, in order to avoid blocking in case the lock is
taken as writer for a long time.
- All other cases can use read_lock().
Fixes: e045199c7c ("evtchn: address races with evtchn_reset()")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
xen/events: fix build
Commit 5f2df45ead ("xen/evtchn: rework per event channel lock")
introduced a build failure for NDEBUG builds.
Fixes: 5f2df45ead ("xen/evtchn: rework per event channel lock")
Signed-off-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
master commit: 5f2df45ead
master date: 2020-11-10 14:36:15 +0100
master commit: 53bacb86f4
master date: 2020-11-11 08:56:21 +0100
Making getBridge() static triggered a build error with some gcc versions:
error: 'strncpy' output may be truncated copying 15 bytes from a string of
length 255 [-Werror=stringop-truncation]
Fix that by using a buffer with 256 bytes instead.
Fixes: 6d0ec05390 ("tools: split libxenstat into new tools/libs/stat directory")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
(cherry picked from commit c8099e48c3)
Today the maximum allowed data length for writing a hypfs node is
tested in the generic hypfs_write() function. For custom runtime
parameters this might be wrong, as the maximum allowed size is derived
from the buffer holding the current setting, while there might be ways
to set the parameter needing more characters than the minimal
representation of that value.
One example for this is the "ept" parameter. Its value buffer is sized
to be able to hold the string "exec-sp=0" or "exec-sp=1", while it is
allowed to use e.g. "no-exec-sp" or "exec-sp=yes" for setting it.
Fix that by moving the length check one level down to the type
specific write function.
In order to avoid allocation of arbitrary sized buffers use a new
MAX_PARAM_SIZE macro as an upper limit for custom writes. The value
of MAX_PARAM_SIZE is the same as the limit in parse_params() for a
single parameter.
Fixes: a659d7cab9 ("xen: add runtime parameter access support to hypfs")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: b4e41b1750
master date: 2020-09-11 14:20:10 +0200
In docs/misc/hypfs-paths.pandoc the supported paths in the hypervisor
file system are specified. Make it more clear that path availability
might change, e.g. due to scope widening or narrowing (e.g. being
limited to a specific architecture).
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Paul Durrant <paul@xen.org>
master commit: 5a4a411bde
master date: 2020-07-20 13:38:00 +0200
The size of the memory allocated for the IOCTL_GNTDEV_MAP_GRANT_REF
ioctl() parameters is calculated wrong, which results in too much
memory allocated.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Release-acked-by: Paul Durrant <paul@xen.org>
Writing the runtime parameters loglvl or guest_loglvl omits setting the
new length of the resulting parameter value.
Reported-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Release-acked-by: Paul Durran <paul@xen.org>
In case of an error in xenhypfs_open() the error path will cause a
segmentation fault due to a wrong sequence of closing calls.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Fixes: 86234eafb9 ("libs: add libxenhypfs")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Release-acked-by: Paul Durrant <paul@xen.org>
Acked-by: Wei Liu <wl@xen.org>
The pv-shim doesn't need the hypervisor filesystem, so disable it.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Commit 58263ed771 ("xen: add /buildinfo/config entry to hypervisor
filesystem") added a dependency to .config, but the hypervisor's build
config could be have another name via setting KCONFIG_CONFIG.
Fix that by using $(KCONFIG_CONFIG) instead. Additionally reference
the config file via $(XEN_ROOT) instead of a relative path.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
As compilers are free to choose the width of an enum they should be
avoided in stable interfaces when declaring a variable. So the
struct xenhypfs_dirent definition should be modified to have explicitly
sized members for type and encoding and the related enums should be
defined separately.
Additionally it is better to have a larger flags member in that struct
with the "writable" indicator occupying only a single bit. This will
make future additions easier.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
Release-acked-by: Paul Durrant <paul@xen.org>
asprintf() can fail, so check its return value. Additionally fix a
memory leak in xenhypfs.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
Release-acked-by: Paul Durrant <paul@xen.org>
xen/tools/binfile contains a bash specific command (let). This leads
to build failures on systems not using bash as /bin/sh.
Replace "let SHIFT=$OPTIND-1" by "SHIFT=$((OPTIND-1))".
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Update connection record details:
- make flags common for sockets and domains (makes it easier to have a
C union for conn-spec)
- add pending incoming data (needed for handling partially read
requests when doing live update)
- add partial response length (needed for proper split to individual
responses after live update)
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Both SHDEPS_libxendevicemodel and SHDEPS_libxenhypfs have a bug by
adding $(SHLIB_xencall) instead of $(SHLIB_libxencall).
The former seems not to have any negative impact, probably because
it is not used anywhere in Xen without the correct $(SHLIB_libxencall)
being used, too.
Fixes: 86234eafb9 ("libs: add libxenhypfs")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Juergen Gross <jgross@suse.com>
The functionality of XEN_SYSCTL_set_parameter is available via hypfs
now, so it can be removed.
This allows to remove the kernel_param structure for runtime parameters
by putting the now only used structure element into the hypfs node
structure of the runtime parameters.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Instead of xc_set_parameters() use xenhypfs_write() for setting
parameters of the hypervisor.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
Add support to read and modify values of hypervisor runtime parameters
via the hypervisor file system.
As runtime parameters can be modified via a sysctl, too, this path has
to take the hypfs rw_lock as writer.
For custom runtime parameters the connection between the parameter
value and the file system is done via an init function which will set
the initial value (if needed) and the leaf properties.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Add the /buildinfo/config entry to the hypervisor filesystem. This
entry contains the .config file used to build the hypervisor.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Provide version and compile information in /buildinfo/ node of the
Xen hypervisor file system. As this information is accessible by dom0
only no additional security problem arises.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
gcc 4.1 has problems with static initializers for anonymous unions.
Fix this by naming the union in struct hypfs_entry_leaf.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Jan Beulich <jbeulich@suse.com>
Add the infrastructure for the hypervisor filesystem.
This includes the hypercall interface and the base functions for
entry creation, deletion and modification.
In order not to have to repeat the same pattern multiple times in case
adding a new node should BUG_ON() failure, the helpers for adding a
node (hypfs_add_dir() and hypfs_add_leaf()) get a nofault parameter
causing the BUG() in case of a failure.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
On the 2019 Xen developer summit there was agreement that the Xen
hypervisor should gain support for a hierarchical name-value store
similar to the Linux kernel's sysfs.
In the beginning there should only be basic support: entries can be
added from the hypervisor itself only, there is a simple hypercall
interface to read the data.
Add a feature document for setting the base of a discussion regarding
the desired functionality and the entries to add.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Add a new script xen/tools/binfile for including a binary file at build
time being usable via a pointer and a size variable in the hypervisor.
Make use of that generic tool in xsm.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Wei Liu <wl@xen.org>
In case opt_ept_ad has not been set explicitly by the user via command
line or runtime parameter, it is treated as "no" on Avoton cpus.
Change that handling by setting opt_ept_ad to 0 for this cpu type
explicitly if no user value has been set.
By putting this into the (renamed) boot time initialization of vmcs.c
_vmx_cpu_up() can be made static.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
The dirty_cpu field of struct vcpu denotes which cpu still holds data
of a vcpu. All accesses to this field should be atomic in case the
vcpu could just be running, as it is accessed without any lock held
in most cases. Especially sync_local_execstate() and context_switch()
for the same vcpu running concurrently have a risk for failing.
There are some instances where accesses are not atomically done, and
even worse where multiple accesses are done when a single one would
be mandated.
Correct that in order to avoid potential problems.
Add some assertions to verify dirty_cpu is handled properly.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
With support of core scheduling sched_unit_migrate_finish() gained a
call of sync_vcpu_execstate() as it was believed to be called as a
result of vcpu migration in any case.
In case of migrating a vcpu away from a physical cpu for a short period
of time but without ever being scheduled on the selected new cpu, this
might not be true so drop the call and let the lazy state syncing do its
job.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Dario Faggioli <dfaggioli@suse.com>
With RCU barriers moved from tasklets to normal RCU processing cpu
offlining in core scheduling might deadlock due to cpu synchronization
required by RCU processing and core scheduling concurrently.
Fix that by bailing out from core scheduling synchronization in case
of pending RCU work. Additionally the RCU softirq is now required to
be of higher priority than the scheduling softirqs in order to do
RCU processing before entering the scheduler again, as bailing out from
the core scheduling synchronization requires to raise another softirq
SCHED_SLAVE, which would bypass RCU processing again.
Reported-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Tested-by: Sergey Dyasli <sergey.dyasli@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Dario Faggioli <dfaggioli@suse.com>
The XS_INTRODUCE command has two parameters: the mfn (or better: gfn)
of the domain's xenstore ring page and the event channel of the
domain for communicating with Xenstore.
The gfn is not really needed. It is stored in the per-domain struct
in xenstored and in case of another XS_INTRODUCE for the domain it
is tested to match the original value. If it doesn't match the
command is aborted via EINVAL, otherwise the event channel to the
domain is recreated.
As XS_INTRODUCE is limited to dom0 and there is no real downside of
recreating the event channel just omit the test for the gfn to
match and don't return EINVAL for multiple XS_INTRODUCE calls.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Acked-by: Wei Liu <wl@xen.org>
credit2 is currently modifying the pause flags of vcpus non-atomically
via sched_set_pause_flags() and sched_clear_pause_flags(). This is
dangerous as there are cases where the paus flags are modified without
any lock held.
So drop the non-atomic pause flag modification functions and rename the
atomic ones dropping the _atomic suffix.
Fixes: a76255b426 ("xen/sched: make credit2 scheduler vcpu agnostic.")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Dario Faggioli <dfaggioli@suse.com>
Commit cb563d7665 ("xen/sched: support core scheduling for moving
cpus to/from cpupools") introduced a regression when trying to remove
an offline cpu from a cpupool, as the system would crash in this
situation.
Fix that by testing the cpu to be online.
Fixes: cb563d7665 ("xen/sched: support core scheduling for moving cpus to/from cpupools")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Dario Faggioli <dfaggioli@suse.com>
The GNTTABOP_cache_flush hypercall has a wrong test for hypercall
continuation, the test today is:
if ( rc > 0 || opaque_out != 0 )
Unfortunately this will be true even in case of an error (rc < 0),
possibly leading to very long lasting hypercalls (times of more
than an hour have been observed in a test case).
Correct the test condition to result in false with rc < 0 and set
opaque_out only if no error occurred, to be on the safe side.
Partially-suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The setup of file descriptors for the Xenstore sockets is needlessly
complicated: the space is allocated dynamically, while two static
variables really would do the job.
For tearing down the sockets it is easier to widen the scope of the
file descriptors from function to file.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Wei Liu <wl@xen.org>
We haven't used mini-os master for about 2 years now due to a stubdom
test failing [1]. Booting a guest with mini-os master used for building
stubdom didn't reveal any problem, so use master for unstable in order
to let OSStest find any problems not showing up in the local test.
[1]: https://lists.xen.org/archives/html/minios-devel/2018-04/msg00015.html
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wl@xen.org>
Commit 562a1c0f7e ("tools/xenstore: dont unlink connection object
twice") introduced a potential use after free problem in
domain_cleanup(): after calling talloc_unlink() for domain->conn
domain->conn is set to NULL. The problem is that domain is registered
as talloc child of domain->conn, so it might be freed by the
talloc_unlink() call.
With Xenstore being single threaded there are normally no concurrent
memory allocations running and freeing a virtual memory area normally
doesn't result in that area no longer being accessible. A problem
could occur only in case either a signal received results in some
memory allocation done in the signal handler (SIGHUP is a primary
candidate leading to reopening the log file), or in case the talloc
framework would do some internal memory allocation during freeing of
the memory (which would lead to clobbering of the freed domain
structure).
Fixes: 562a1c0f7e ("tools/xenstore: dont unlink connection object twice")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
With commit cef21210fb ("rcu: don't process callbacks when holding
a rcu_read_lock()") the comment in process_pending_softirqs() about
not entering the scheduler should have been moved.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Xen's RCU implementation relies on no softirq handling taking place
while being in a RCU critical section. Add ASSERT()s in debug builds
in order to catch any violations.
For that purpose modify rcu_read_[un]lock() to use a dedicated percpu
counter additional to preempt_[en|dis]able() as this enables to test
that condition in __do_softirq() (ASSERT_NOT_IN_ATOMIC() is not
usable there due to __cpu_up() calling process_pending_softirqs()
while holding the cpu hotplug lock).
While at it switch the rcu_read_[un]lock() implementation to static
inline functions instead of macros.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Some keyhandlers are calling process_pending_softirqs() while holding
a rcu_read_lock(). This is wrong, as process_pending_softirqs() might
activate rcu calls which should not happen inside a rcu_read_lock().
For that purpose modify process_pending_softirqs() to not allow rcu
callback processing when a rcu_read_lock() is being held.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Today rcu_barrier() is calling stop_machine_run() to synchronize all
physical cpus in order to ensure all pending rcu calls have finished
when returning.
As stop_machine_run() is using tasklets this requires scheduling of
idle vcpus on all cpus imposing the need to call rcu_barrier() on idle
cpus only in case of core scheduling being active, as otherwise a
scheduling deadlock would occur.
There is no need at all to do the syncing of the cpus in tasklets, as
rcu activity is started in __do_softirq() called whenever softirq
activity is allowed. So rcu_barrier() can easily be modified to use
softirq for synchronization of the cpus no longer requiring any
scheduling activity.
As there already is a rcu softirq reuse that for the synchronization.
Remove the barrier element from struct rcu_data as it isn't used.
Finally switch rcu_barrier() to return void as it now can never fail.
Partially-based-on-patch-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
When using atomic variables for synchronization barriers are needed
to ensure proper data serialization. Introduce smp_mb__before_atomic()
and smp_mb__after_atomic() as in the Linux kernel for that purpose.
Use the same definitions as in the Linux kernel.
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Offlining a cpu with core scheduling active can result in a hanging
system. Reason is the scheduling resource and unit of the to be removed
cpus needs to be split in order to remove the cpu from its cpupool and
move it to the idle scheduler. In case one of the involved cpus happens
to have received a sched slave event due to a vcpu former having been
running on that cpu being woken up again, it can happen that this cpu
will enter sched_wait_rendezvous_in() while its scheduling resource is
just about to be split. It might wait for ever for the other sibling
to join, which will never happen due to the resources already being
modified.
This can easily be avoided by:
- resetting the rendezvous counters of the idle unit which is kept
- checking for a new scheduling resource in sched_wait_rendezvous_in()
after reacquiring the scheduling lock and resetting the counters in
that case without scheduling another vcpu
- moving schedule resource modifications (in schedule_cpu_rm()) and
retrieving (schedule(), sched_slave() is fine already, others are not
critical) into locked regions
Reported-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Dario Faggioli <dfaggioli@suse.com>
When onlining a cpu cpupool_cpu_add() checks whether all siblings of
the new cpu are free in order to decide whether to add it to cpupool0.
In case the added cpu is not the last sibling to be onlined this test
is wrong as it only checks for all online siblings to be free. The
test should include the check for the number of siblings having
reached the scheduling granularity of cpupool0, too.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Dario Faggioli <dfaggioli@suse.com>
In case Xen ever gains preemption support the spinlock coding's
placement of preempt_disable() and preempt_enable() should be outside
of the locked section.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Similar to spinlocks preemption should be disabled while holding a
rwlock.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
rcu_lock_domain() misuses the domain structure as rcu lock, which is
working only as long as rcu_read_lock() isn't evaluating the lock.
Fix that by adding a rcu lock to struct domain and use that for
rcu_lock_domain().
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
As rcu callbacks are processed in __do_softirq() there is no need to
use the scheduling softirq for forcing quiescent state. Any other
softirq would do the job and the scheduling one is the most expensive.
So use the already existing rcu softirq for that purpose. For telling
apart why the rcu softirq was raised add a flag for the current usage.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Support of other variable sizes than that of normal bool ones for
boolean_param() don't make sense, so catch any other sized variables
at build time.
Fix the one parameter using a plain int instead of bool.
Signed-off-by: Juergen Gross <jgross@suse.com>
[add __read_mostly]
Reviewed-by: Jan Beulich <jbeulich@suse.com>
In case moving away all domains from the cpu to be removed is failing
in cpupool_unassign_cpu_start() the error path is missing to release
sched_res_rculock.
The normal exit path is releasing domlist_read_lock instead (this is
currently no problem as the reference to the specific rcu lock is not
used by rcu_read_unlock()).
While at it indent the present error label by one space.
Reported-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Dario Faggioli <dfaggioli@suse.com>
Juergen Gross