Commit Graph
28861 Commits
Author SHA1 Message Date
Thomas Sanders 6bf0560e12 oxenstored: trim history in the frequent_ops function
We were trimming the history of commits only at the end of each
transaction (regardless of how it ended).

Therefore if non-transactional writes were being made but no
transactions were being ended, the history would grow
indefinitely. Now we trim the history at regular intervals.

Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
2017-04-05 15:26:38 +01:00
Thomas Sanders ebd64cd82a oxenstored transaction conflicts: improve logging
For information related to transaction conflicts, potentially frequent
logging at "info" priority has been changed to "debug" priority, and
once per two minutes there is an "info" priority summary.

Additional detailed logging has been added at "debug" priority.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
2017-04-05 15:26:38 +01:00
Thomas Sanders 442df65d24 oxenstored: don't wake to issue no conflict-credit
In the main loop, when choosing the timeout for the select function
call, we were setting it so as to wake up to issue conflict-credit to
any domains that could accept it. When xenstore is idle, this would
mean waking up every 50ms (by default) to do no work. With this
commit, we check whether any domain is below its cap, and if not then
we set the timeout for longer (the same timeout as before the
conflict-protection feature was added).

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
Reviewed-by: Jonathan Davies <jonathan.davies@citrix.com>
2017-04-05 15:26:38 +01:00
Thomas Sanders 63e3e0b1e3 oxenstored: do not commit read-only transactions
The packet telling us to end the transaction has always carried an
argument telling us whether to commit.

If the transaction made no modifications to the tree, now we ignore
that argument and do not commit: it is just a waste of effort.

This makes read-only transactions immune to conflicts, and means that
we do not need to store any of their details in the history that is
used for assigning blame for conflicts.

We count a transaction as a read-only transaction only if it contains
no operations that modified the tree.

This means that (for example) a transaction that creates a new node
then deletes it would NOT count as read-only, even though it makes no
change overall. A more sophisticated algorithm could judge the
transaction based on comparison of its initial and final states, but
this would add complexity and computational cost.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
Reviewed-by: Jonathan Davies <jonathan.davies@citrix.com>
2017-04-05 15:26:38 +01:00
Thomas Sanders e0fb21c2bc oxenstored: allow self-conflicts
We already avoid inter-domain conflicts but now allow intra-domain
conflicts.  Although there are no known practical examples of a domain
that might perform operations that conflict with its own transactions,
this is conceivable, so here we avoid changing those semantics
unnecessarily.

When a transaction commit fails with a conflict and we look through
the history of commits to see which connection(s) to blame, ignore
historical commits that were made by the same connection as the
failing commit.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
Reviewed-by: Jonathan Davies <jonathan.davies@citrix.com>
2017-04-05 15:26:38 +01:00
Jonathan Davies fd5dfb6c9f oxenstored: blame the connection that caused a transaction conflict
Blame each connection found to have made a commit that would cause this
transaction to fail. Each blamed connection is penalised by having its
conflict-credit decremented.

Note the change in semantics for the replay function: we no longer stop after
finding the first operation that can't be replayed. This allows us to identify
all operations that conflicted with this transaction, not just the one that
conflicted first.

Signed-off-by: Jonathan Davies <jonathan.davies@citrix.com>
Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
v1 Reviewed-by: Christian Lindig <christian.lindig@citrix.com>

Changes since v1:
 * use correct log levels for informational messages
Changes since v2:
 * fix the blame algorithm and improve logging
   (fix was reviewed by Jonathan Davies)

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
2017-04-05 15:26:38 +01:00
Jonathan Davies b217630a0e oxenstored: track commit history
Since the list of historic activity cannot grow without bound, it is safe to use
this to track commits.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Jonathan Davies <jonathan.davies@citrix.com>
Reviewed-by: Thomas Sanders <thomas.sanders@citrix.com>
2017-04-05 15:26:38 +01:00
Thomas Sanders 40ffd04425 oxenstored: discard old commit-history on txn end
The history of commits is to be used for working out which historical
commit(s) (including atomic writes) caused conflicts with a
currently-failing commit of a transaction. Any commit that was made
before the current transaction started cannot be relevant. Therefore
we never need to keep history from before the start of the
longest-running transaction that is open at any given time: whenever a
transaction ends (with or without a commit) then if it was the
longest-running open transaction we can delete history up until start
of the the next-longest-running open transaction.

Some transactions might stay open for a very long time, so if any
transaction exceeds conflict_max_history_seconds then we remove it
from consideration in this context, and will not guarantee to keep
remembering about historical commits made during such a transaction.

We implement this by keeping a list of all open transactions that have
not been open too long. When a transaction ends, we remove it from the
list, along with any that have been open longer than the maximum; then
we delete any history from before the start of the longest-running
transaction remaining in the list.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
Reviewed-by: Jonathan Davies <jonathan.davies@citrix.com>
Reviewed-by: Christian Lindig <christian.lindig@citrix.com>
2017-04-05 15:26:38 +01:00
Jonathan Davies 2547b6133e oxenstored: only record operations with side-effects in history
There is no need to record "read" operations as they will never cause another
transaction to fail.

Signed-off-by: Jonathan Davies <jonathan.davies@citrix.com>
Reviewed-by: Thomas Sanders <thomas.sanders@citrix.com>

Backport 4.6 -> 4.5 by removing reference to XS_RESET_WATCHES.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
2017-04-05 15:26:38 +01:00
Jonathan Davies 69950a74a2 oxenstored: support commit history tracking
Add ability to track xenstore tree operations -- either non-transactional
operations or committed transactions.

For now, the call to actually retain commits is commented out because history
can grow without bound.

For now, we call record_commit for all non-transactional operations. A
subsequent patch will make it retain only the ones with side-effects.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Jonathan Davies <jonathan.davies@citrix.com>
Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
Reviewed-by: Christian Lindig <christian.lindig@citrix.com>
2017-04-05 15:26:38 +01:00
Jonathan Davies 5c4b09b6bd oxenstored: add transaction info relevant to history-tracking
Specifically:
 * retain the original store (not just the root) in full transactions
 * store commit count at the time of the start of the transaction

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Jonathan Davies <jonathan.davies@citrix.com>
Reviewed-by: Thomas Sanders <thomas.sanders@citrix.com>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Christian Lindig <christian.lindig@citrix.com>
2017-04-05 15:26:38 +01:00
Thomas Sanders faf0314c4f oxenstored: ignore domains with no conflict-credit
When processing connections, skip those from domains with no remaining
conflict-credit.

Also, issue a point of conflict-credit at regular intervals, the
period being set by the configuration option "conflict-max-history-
seconds".  When issuing conflict-credit, we give a point either to
every domain at once (one each) or only to the single domain at the
front of the queue, depending on the configuration option
"conflict-rate-limit-is-aggregate".

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
Reviewed-by: Jonathan Davies <jonathan.davies@citrix.com>
Reviewed-by: Christian Lindig <christian.lindig@citrix.com>
2017-04-05 15:26:38 +01:00
Thomas Sanders 2e7a50f487 oxenstored: handling of domain conflict-credit
This commit gives each domain a conflict-credit variable, which will
later be used for limiting how often a domain can cause other domain's
transaction-commits to fail.

This commit also provides functions and data for manipulating domains
and their conflict-credit, and checking whether they have credit.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
Reviewed-by: Jonathan Davies <jonathan.davies@citrix.com>
Reviewed-by: Christian Lindig <christian.lindig@citrix.com>
2017-04-05 15:26:38 +01:00
Thomas Sanders ec68489c4a oxenstored: comments explaining some variables
It took a while of reading and reasoning to work out what these are
for, so here are comments to make life easier for everyone reading
this code in future.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
Reviewed-by: Jonathan Davies <jonathan.davies@citrix.com>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Christian Lindig <christian.lindig@citrix.com>
2017-04-05 15:26:38 +01:00
Jonathan Davies 24d21875e7 oxenstored: allow compilation prior to OCaml 3.12.0
Commit 363ae55c8 used an OCaml feature called record field punning. This broke
the build on compilers prior to OCaml 3.12.0.

This patch makes no semantic change but now uses backwards-compatible syntax.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Jonathan Davies <jonathan.davies@citrix.com>
Reported-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Tested-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
2017-04-05 15:26:38 +01:00
Jonathan Davies 28654eb736 oxenstored: log request and response during transaction replay
During a transaction replay, the replayed requests and the new responses are
logged in the same way as the original requests and the original responses.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Jonathan Davies <jonathan.davies@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jon Ludlam <jonathan.ludlam@citrix.com>
Reviewed-by: Euan Harris <euan.harris@citrix.com>
Acked-by: David Scott <dave@recoil.org>
2017-04-05 15:26:38 +01:00
Jonathan Davies db2154af50 oxenstored: replay transaction upon conflict
The existing transaction merge algorithm keeps track of the least upper bound
(longest common prefix) of all the nodes which have been read and written, and
will re-combine two stores which have disjoint upper bounds. This works well for
small transactions but causes unnecessary conflicts for ones that span a large
subtree, such as the following ones used by the xapi toolstack:

 * VM start: creates /vm/... /vss/... /local/domain/...
   The least upper bound of this transaction is / and so all
   these transactions conflict with everything.

 * Device hotplug: creates /local/domain/0/... /local/domain/n/...
   The least upper bound of this transaction is /local/domain so
   all these transactions conflict with each other.

If the existing merge algorithm cannot merge and commit, we attempt
a /replay/ of the failed transaction against the new store.

When we replay the requests we check whether the response sent to the client is
the same as during the first attempt at the transaction. If the responses are
all the same then the transaction replay can be committed. If any differ then
the transaction replay must be aborted and the client must retry.

This algorithm uses the intuition that the transactions made by the toolstack
are designed to be for separate domains, and should fundamentally not conflict
in the sense that they don't read or write any shared keys. By replaying the
transaction on the server side we do what the client would have to do anyway,
only we can do it quickly without allowing any other requests to interfere.

Performing 300 parallel simulated VM start and shutdowns without this code:

300 parallel starts and shutdowns: 268.92

Performing 300 parallel simulated VM start and shutdowns with this code:

300 parallel starts and shutdowns: 3.80

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Dave Scott <dave@recoil.org>
Signed-off-by: Jonathan Davies <jonathan.davies@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jon Ludlam <jonathan.ludlam@citrix.com>
Reviewed-by: Euan Harris <euan.harris@citrix.com>
Acked-by: David Scott <dave@recoil.org>
2017-04-05 15:26:37 +01:00
Jonathan Davies 5c1d22f7d7 oxenstored: move functions that process simple operations
Separate the functions which process operations that can be done as part of a
transaction. Specifically, these operations are: read, write, rm, getperms,
setperms, getdomainpath, directory, mkdir.

Also split function_of_type into two functions: one for processing the simple
operations and one for processing the rest.

This will help allow replay of transactions, allowing us to invoke the functions
that process the simple operations as part of the processing of transaction_end.

Signed-off-by: Jonathan Davies <jonathan.davies@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jon Ludlam <jonathan.ludlam@citrix.com>
Reviewed-by: Euan Harris <euan.harris@citrix.com>
Acked-by: David Scott <dave@recoil.org>

Backporting to 4.5:

- Removed references to Reset_watches, which was introduced in 4.6.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
2017-04-05 15:26:37 +01:00
Jonathan Davies aa5d9c9fae oxenstored: keep track of each transaction's operations
A list of (request, response) pairs from the operations performed within the
transaction will be useful to support transaction replay.

Since this consumes memory, the number of requests per transaction must not be
left unbounded. Hence a new quota for this is introduced. This quota, configured
via the configuration key 'quota-maxrequests', limits the size of transactions
initiated by domUs.

After the maximum number of requests has been exhausted, any further requests
will result in EQUOTA errors. The client may then choose to end the transaction;
a successful commit will result in the retention of only the prior requests.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Jonathan Davies <jonathan.davies@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jon Ludlam <jonathan.ludlam@citrix.com>
Reviewed-by: Euan Harris <euan.harris@citrix.com>
Acked-by: David Scott <dave@recoil.org>
2017-04-05 15:26:37 +01:00
Jonathan Davies 19b1599ae3 oxenstored: refactor request processing
Encapsulate the request in a record that is passed from do_input to
process_packet and input_handle_error.

This will be helpful when keeping track of the requests made as part of a
transaction.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Jonathan Davies <jonathan.davies@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jon Ludlam <jonathan.ludlam@citrix.com>
Reviewed-by: Euan Harris <euan.harris@citrix.com>
Acked-by: David Scott <dave@recoil.org>
2017-04-05 15:26:37 +01:00
Jonathan Davies a99b6600b2 oxenstored: remove some unused parameters
Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Jonathan Davies <jonathan.davies@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jon Ludlam <jonathan.ludlam@citrix.com>
Reviewed-by: Euan Harris <euan.harris@citrix.com>
Acked-by: David Scott <dave@recoil.org>
2017-04-05 15:26:37 +01:00
Jonathan Davies 262754b45e oxenstored: refactor putting response on wire
Previously, the functions reply_{ack,data,data_or_ack} and input_handle_error
put the response on the wire by invoking Connection.send_{ack,reply,error}.

Instead, these functions now return a value indicating what needs to be put on
the wire, and that action is done by a send_response function called
afterwards.

This refactoring gives us a chance to store the value of the response, useful
for replaying transactions.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Jonathan Davies <jonathan.davies@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jon Ludlam <jonathan.ludlam@citrix.com>
Reviewed-by: Euan Harris <euan.harris@citrix.com>
Acked-by: David Scott <dave@recoil.org>
2017-04-05 15:26:37 +01:00
Zheng Li 7e68f13c3b oxenstored: add a safe net mechanism for existing ill-behaved clients
In previous commit, we moved from exhaustively scanning all domain connections
to only processing those have correctly notified us by events. The benefits are
not only efficiency but also correctness, because it could potentially block an
ill-behaved client and have it waiting on its own mistake. If someone makes a
mistake on this when developing a piece of code, he/she would immediately
notice the problem (as the process being blocked), so that he/she could fix it
rightaway before anything else. Note that the chances of making such mistakes
are rare in reality, because most client code would use the libxenstore library
(which has all the notification logic built in correctly) instead of having to
implement raw accessing from scratch.

On the other hand, we did notice that there were some legacy code that didn't do
the notification correctly. As some code might be still running in wild, it
would be bad if they break by this change (e.g. after an upgrade). This patch
introduces a safe net mechanism to ensure ill-behaved clients continue to work,
but still retain most of the performance benefits here.

  * We add a checker to still scan all the rings periodically, so that we can
    still pick up these messages at an acceptable frequency.

  * Internally, we introduce an io_credit concept for domain connections. It
    represents the rounds of ring scan we are going to perform on a domain
    connection. For well-behaved connections, this value is changing between 0
    and 1; but for connections detected as ill-behaved, we'll bump its credit
    to a high value so that we'll unconditionally scan its ring for the next
    $n$ rounds. This way, the client won't hiccupped by the interval between
    checker's running (especially during periods when it continously interacts
    with oxenstored); and oxenstored doesn't have to keep scanning these
    rings indefinitely (with the credit running out), as they are usually quite
    most of the time.

  * We log an message when a domain connection is suspected as ill-behaved.
    Enable [info] level logging if you want/need to see it in action. Note that
    this information won't be accurate, as false positives are possible due to
    time window (e.g. we detect a client has written to the ring and we get no
    notificiation from it for the time being, but still the notification could
    potentially arrive at some time later). It's no harm to give a domain
    connection extra credit though.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Zheng Li <dev@zheng.li>
Reviewed-by: David Scott <dave.scott@citrix.com>
2017-04-05 15:26:37 +01:00
Zheng Li 04386b51c6 oxenstored: only process domain connections that notify us by events
Currently, upon receiving an event, oxenstored will always scan/process all
the domain connections (xs rings), disregarding which domain sent that event.
This is rather costy and inefficient. It also shadows and indulges client
for not correctly communicating with us on message/space availability.

With this patch, oxenstore will only scan/process the domain connections
that have correctly notified us by events or have IO actions leftover from
previous communication.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Zheng Li <dev@zheng.li>
Reviewed-by: David Scott <dave.scott@citrix.com>
2017-04-05 15:26:37 +01:00
Zheng Li 1fb43d3e7c oxenstored: enable domain connection indexing based on eventchn port
Currently in xenstore connection database,  we use a hash table of
(domid -> connection) to store domain connections. This allows fast indexing
based on dom ids.

This patch adds another dimention of fast indexing that is based on eventchn
port number. This is useful when doing selective connection processing
based on the port numbers of incoming events.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Zheng Li <dev@zheng.li>
Reviewed-by: David Scott <dave.scott@citrix.com>
2017-04-05 15:26:37 +01:00
Zheng Li 0a8a4af826 oxenstored: use hash table to store socket connections
Currently we use list to store socket connections. This is fine for smaller
number of connections. But when we scale up, traveling through a list of
hundreds or thousands of connections just to find a single one of them is very
low efficient.

This patch replaces the list with a (Unix.file_descr -> Connection.t) hash table.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Zheng Li <dev@zheng.li>
Reviewed-by: David Scott <dave.scott@citrix.com>
2017-04-05 15:26:37 +01:00
Zheng Li edfddb3da8 oxenstored: catch the error when a connection is already deleted
The function process_fdset_with is called on the read set connections first.
During the process, it might destroy a connection and remove it from the
connections database if some errors occur. However, a reference to the same
connection might still exist in the write set, which is awaiting to be
processed next. In this case, a Not_found error will be raised and the process
is aborted.

This patch changes the logic to ignore connections just missing from the
connection database and continue the rest part of the work.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Zheng Li <dev@zheng.li>
Reviewed-by: David Scott <dave.scott@citrix.com>
2017-04-05 15:26:37 +01:00
Jerome Maloberti 53a6387e03 oxenstored: perform a 3-way merge of the quota after a transaction
At a beginning of a transaction, the quotas from the global store
are duplicated and modified by the transaction. If during the
transaction, an action associated to no transaction is concurrently
executed, the quotas of the global store are updated, and then the
updates are lost when the transaction merges.

We fix this problem by keeping another copy of the quota at the
beginning of the transaction, and performing a 3-way merge between
the quotas from the transaction and the "original" copy of the quota
onto the quota of the global store.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Jerome Maloberti <jerome.maloberti@citrix.com>
Signed-off-by: Euan Harris <euan.harris@citrix.com>
Acked-by: David Scott <dave.scott@citrix.com>
2017-04-05 15:26:37 +01:00
Vincent Bernardoff e6b2014af5 oxenstored: exempt dom0 from domU node quotas
If a domU has exhausted its quota we still want the toolstack in dom0 to
be able to create new nodes in places like
  /local/domain/%d/control/shutdown

Without this patch, a domU which has exhausted its quota can only be
powered off, which is not as good as being able to request a clean
shutdown.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
Signed-off-by: Euan Harris <euan.harris@citrix.com>
Acked-by: David Scott <dave.scott@citrix.com>
2017-04-05 15:26:37 +01:00
Ian Jackson 3d00b3e8ac xenstored: Log when the write transaction rate limit bites
Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

plus:

xenstore: dont increment bool variable
Instead of incrementing a bool variable just set it to true.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2017-04-05 15:26:29 +01:00
Ian Jackson 3c7504e7d3 xenstored: apply a write transaction rate limit
This avoids a rogue client being about to stall another client (eg the
toolstack) indefinitely.

This is XSA-206.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
2017-04-05 15:26:21 +01:00
Jan Beulich 89bd1cf127 memory: properly check guest memory ranges in XENMEM_exchange handling
The use of guest_handle_okay() here (as introduced by the XSA-29 fix)
is insufficient here, guest_handle_subrange_okay() needs to be used
instead.

Note that the uses are okay in
- XENMEM_add_to_physmap_batch handling due to the size field being only
  16 bits wide,
- livepatch_list() due to the limit of 1024 enforced on the
  number-of-entries input (leaving aside the fact that this can be
  called by a privileged domain only anyway),
- compat mode handling due to counts there being limited to 32 bits,
- everywhere else due to guest arrays being accessed sequentially from
  index zero.

This is CVE-2017-7228 / XSA-212.

Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 938fd2586e
master date: 2017-04-04 14:47:46 +0200
2017-04-04 15:06:29 +02:00
Ian Jackson 9744389956 QEMU_TAG update 2017-03-21 18:45:09 +00:00
Ian Jackson 5e0448f4bb QEMU_TAG update 2017-02-22 16:46:48 +00:00
Oleksandr Tyshchenko b648113f8a IOMMU: always call teardown callback
There is a possible scenario when (d)->need_iommu remains unset
during guest domain execution. For example, when no devices
were assigned to it. Taking into account that teardown callback
is not called when (d)->need_iommu is unset we might have unreleased
resourses after destroying domain.

So, always call teardown callback to roll back actions
that were performed in init callback.

This is XSA-207.

Signed-off-by: Oleksandr Tyshchenko <olekstysh@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Julien Grall <julien.grall@arm.com>
2017-02-15 12:21:10 +00:00
Jan Beulich 394ddc2de6 x86: force EFLAGS.IF on when exiting to PV guests
Guest kernels modifying instructions in the process of being emulated
for another of their vCPU-s may effect EFLAGS.IF to be cleared upon
next exiting to guest context, by converting the being emulated
instruction to CLI (at the right point in time). Prevent any such bad
effects by always forcing EFLAGS.IF on. And to cover hypothetical other
similar issues, also force EFLAGS.{IOPL,NT,VM} to zero.

This is CVE-2016-10024 / XSA-202.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 0e47f92b07
master date: 2016-12-21 16:46:13 +0100
2016-12-21 17:48:43 +01:00
Andrew Cooper 5a343e41f5 x86/emul: Correct the handling of eflags with SYSCALL
A singlestep #DB is determined by the resulting eflags value from the
execution of SYSCALL, not the original eflags value.

By using the original eflags value, we negate the guest kernels attempt to
protect itself from a privilege escalation by masking TF.

Introduce a tf boolean and have the SYSCALL emulation recalculate it
after the instruction is complete.

This is XSA-204

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2016-12-19 15:46:47 +00:00
Jan Beulich 619db7dffb x86emul: CMPXCHG8B ignores operand size prefix
Otherwise besides mis-handling the instruction, the comparison failure
case would result in uninitialized stack data being handed back to the
guest in rDX:rAX (32 bits leaked for 32-bit guests, 96 bits for 64-bit
ones).

This is CVE-2016-9932 / XSA-200.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
2016-12-13 14:30:14 +01:00
Ian Jackson 149c34a6ea QEMU_TAG update 2016-12-07 16:54:47 +00:00
Ian Jackson 1c1bfc1cf2 QEMU_TAG update 2016-11-29 18:40:26 +00:00
Ian Jackson 6639a202f2 pygrub: Properly quote results, when returning them to the caller:
* When the caller wants sexpr output, use `repr()'
  This is what Xend expects.

  The returned S-expressions are now escaped and quoted by Python,
  generally using '...'.  Previously kernel and ramdisk were unquoted
  and args was quoted with "..." but without proper escaping.  This
  change may break toolstacks which do not properly dequote the
  returned S-expressions.

* When the caller wants "simple" output, crash if the delimiter is
  contained in the returned value.

  With --output-format=simple it does not seem like this could ever
  happen, because the bootloader config parsers all take line-based
  input from the various bootloader config files.

  With --output-format=simple0, this can happen if the bootloader
  config file contains nul bytes.

This is CVE-2016-9379 and CVE-2016-9380 / XSA-198.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Tested-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 27e14d346e
master date: 2016-11-22 13:52:09 +0100
2016-11-22 14:35:31 +01:00
Jan Beulich 14fa85aab3 x86emul: fix huge bit offset handling
We must never chop off the high 32 bits.

This is CVE-2016-9383 / XSA-195.

Reported-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 1c6c2d60d2
master date: 2016-11-22 13:49:06 +0100
2016-11-22 14:34:56 +01:00
Jan Beulich 1827d5252c x86/PV: writes of %fs and %gs base MSRs require canonical addresses
Commit c42494acb2 ("x86: fix FS/GS base handling when using the
fsgsbase feature") replaced the use of wrmsr_safe() on these paths
without recognizing that wr{f,g}sbase() use just wrmsrl() and that the
WR{F,G}SBASE instructions also raise #GP for non-canonical input.

Similarly arch_set_info_guest() needs to prevent non-canonical
addresses from getting stored into state later to be loaded by context
switch code. For consistency also check stack pointers and LDT base.
DR0..3, otoh, already get properly checked in set_debugreg() (albeit
we discard the error there).

The SHADOW_GS_BASE check isn't strictly necessary, but I think we
better avoid trying the WRMSR if we know it's going to fail.

This is CVE-2016-9385 / XSA-193.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: f3fa3abf3e
master date: 2016-11-22 13:46:28 +0100
2016-11-22 14:34:25 +01:00
Jan Beulich e8a46a2104 x86/HVM: don't load LDTR with VM86 mode attrs during task switch
Just like TR, LDTR is purely a protected mode facility and hence needs
to be loaded accordingly. Also move its loading to where it
architecurally belongs.

This is CVE-2016-9382 / XSA-192.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 93aa42b85a
master date: 2016-11-22 13:45:44 +0100
2016-11-22 14:33:54 +01:00
Andrew Cooper ed77368cc9 x86/hvm: Fix the handling of non-present segments
In 32bit, the data segments may be NULL to indicate that the segment is
ineligible for use.  In both 32bit and 64bit, the LDT selector may be NULL to
indicate that the entire LDT is ineligible for use.  However, nothing in Xen
actually checks for this condition when performing other segmentation
checks.  (Note however that limit and writeability checks are correctly
performed).

Neither Intel nor AMD specify the exact behaviour of loading a NULL segment.
Experimentally, AMD zeroes all attributes but leaves the base and limit
unmodified.  Intel zeroes the base, sets the limit to 0xfffffff and resets the
attributes to just .G and .D/B.

The use of the segment information in the VMCB/VMCS is equivalent to a native
pipeline interacting with the segment cache.  The present bit can therefore
have a subtly different meaning, and it is now cooked to uniformly indicate
whether the segment is usable or not.

GDTR and IDTR don't have access rights like the other segments, but for
consistency, they are treated as being present so no special casing is needed
elsewhere in the segmentation logic.

AMD hardware does not consider the present bit for %cs and %tr, and will
function as if they were present.  They are therefore unconditionally set to
present when reading information from the VMCB, to maintain the new meaning of
usability.

Intel hardware has a separate unusable bit in the VMCS segment attributes.
This bit is inverted and stored in the present field, so the hvm code can work
with architecturally-common state.

This is CVE-2016-9386 / XSA-191.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 04beafa8e6
master date: 2016-11-22 13:44:50 +0100
2016-11-22 14:32:39 +01:00
Jan Beulich 488b7d28ab x86emul: honor guest CR0.TS and CR0.EM
We must not emulate any instructions accessing respective registers
when either of these flags is set in the guest view of the register, or
else we may do so on data not belonging to the guest's current task.

Being architecturally required behavior, the logic gets placed in the
instruction emulator instead of hvmemul_get_fpu(). It should be noted,
though, that hvmemul_get_fpu() being the only current handler for the
get_fpu() callback, we don't have an active problem with CR4: Both
CR4.OSFXSR and CR4.OSXSAVE get handled as necessary by that function.

This is XSA-190.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2016-10-04 14:06:16 +01:00
Jan Beulich dfddbf35d9 evtchn-fifo: prevent use after free
evtchn_fifo_init_control() calls evtchn_fifo_destroy() on an error
path, leading to cleanup_event_array() which frees d->evtchn_fifo
without also clearing the pointer. Otoh the bulk of
evtchn_fifo_init_control() is dependent on d->evtchn_fifo being NULL.

This is XSA-188 / CVE-2016-7154.

Reported-by: Mikhail V Gorobets <mikhail.v.gorobets@intel.com>
Suggested-by: Mikhail V Gorobets <mikhail.v.gorobets@intel.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2016-09-08 14:32:51 +02:00
Andrew Cooper 2f3e08d37b x86/shadow: Avoid overflowing sh_ctxt->seg_reg[]
hvm_get_seg_reg() does not perform a range check on its input segment, calls
hvm_get_segment_register() and writes straight into sh_ctxt->seg_reg[].

x86_seg_none is outside the bounds of sh_ctxt->seg_reg[], and will hit a BUG()
in {vmx,svm}_get_segment_register().

HVM guests running with shadow paging can end up performing a virtual to
linear translation with x86_seg_none.  This is used for addresses which are
already linear.  However, none of this is a legitimate pagetable update, so
fail the emulation in such a case.

This is XSA-187 / CVE-2016-7094.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Tim Deegan <tim@xen.org>
master commit: a9f3b3bad1
master date: 2016-09-08 14:16:26 +0200
2016-09-08 14:32:16 +02:00
Jan Beulich fbe0fb4b52 x86/32on64: don't allow recursive page tables from L3
L3 entries are special in PAE mode, and hence can't reasonably be used
for setting up recursive (and hence linear) page table mappings. Since
abuse is possible when the guest in fact gets run on 4-level page
tables, this needs to be excluded explicitly.

This is XSA-185 / CVE-2016-7092.

Reported-by: Jérémie Boutoille <jboutoille@ext.quarkslab.com>
Reported-by: "栾尚聪(好风)" <shangcong.lsc@alibaba-inc.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: c844d637d9
master date: 2016-09-08 14:14:53 +0200
2016-09-08 14:30:11 +02:00
Andrew Cooper 0fe7d69617 x86/pv: Remove unsafe bits from the mod_l?_entry() fastpath
All changes in writeability and cacheability must go through full
re-validation.

Rework the logic as a whitelist, to make it clearer to follow.

This is XSA-182

Reported-by: Jérémie Boutoille <jboutoille@ext.quarkslab.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Tim Deegan <tim@xen.org>
2016-07-26 14:11:29 +01:00