Merge version 4.16.2-1+rpi1 and 4.16.2+90-g0d39a6d1ae-1 to produce 4.16.2+90-g0d39a6d1ae-1+rpi1

Declare fast forward / record previous work

[git-debrebase pseudomerge: stitch]

Commit patch queue (exported by git-debrebase)

[git-debrebase make-patches: export and commit patches]

debian/changelog: finish 4.16.2+90-g0d39a6d1ae-1

libxl: Fix unneededly rebuilding build.o(pic)

[The symptoms]

When doing a Xen package build for Debian with ccache enabled, we
started getting the following error:

    x86_64-linux-gnu-gcc [...] -o build.o
    /builds/xen-team/debian-xen/debian/output/source_dir/tools/libs/light/../../../tools/libacpi/build.c
    ccache: error: Failed to create temporary file for
    /run/user/0/ccache-tmp/tmp.cpp_stdout.bqxKOP: Permission denied

It turns out to be the case that during the install step of tools (the
install-tools that happens inside the override_dh_auto_install part of
d/rules), the upstream build machinery *again* tries to build this
build.c file, while this has already been done earlier during the actual
build phase.

Since the Debian build process stopped to allow usage of ccache during
the install phase of the process, this issue surfaces.

[The cause]

In tools/libs/light/Makefile, we see the following lines:

    .PHONY: acpi
    acpi:
        $(MAKE) -C $(ACPI_PATH) ACPI_BUILD_DIR=$(CURDIR) DSDT_FILES="$(DSDT_FILES-y)"

    [...]

    $(DSDT_FILES-y) build.o build.opic: acpi

'acpi' is defined as phony target. In the last line, we see that build.o
depdends on acpi.

Also see:
    "4.6 Phony Targets"
    https://www.gnu.org/software/make/manual/make.html#Phony-Targets

A 'normal' target gives make the possibility to track timestamps of a
target file. E.g. compiling foo.c results in foo.o, and as long as foo.c
keeps being 'older' than foo.o, make will think "nothing to do here,
foo.o is up to date, let's move along".

Now, a phony target is some kind of fake target that does not come with
this kind of information, and such behaves like a target that is always
out-of-date. Hence, with a configuration as seen above, it will try to
always unneededly build this build.o and build.opic again.

[Discussion]

Upstream commit e006b2e3be ("libxl: fix libacpi dependency") which
introduced the problem tells us that the purpose of the current
configuration is to make sure the libacpi/ dir is built before we
attempt to work on build.c in here. The changes in there remove an
apparently obsolete line referencing build.o from the libacpi Makefile,
which might mean that in the past this build.* stuff was located in that
part of the code, and was moved into libs/light later.

[The fix]

If it is enough to just have an order-only dependency, we can use an
order-only prerequisite instead, in this place:

    $(DSDT_FILES-y): acpi
    build.o build.opic: | acpi

Also see:
    "4.3 Types of Prerequisites"
    https://www.gnu.org/software/make/manual/make.html#Prerequisite-Types

Now the build machinery will not attempt to unconditionally rebuild
build.o during make install.

[Suggestions for further work]

As can be seen, there's still the $(DSDT_FILES-y) which has the same
acpi dependency and which may lead to similar unwanted side effects.
However, since none of the files in that list have a corresponding build
target in *this* Makefile, it does not trigger the problem for us, and
we leave it alone, for now.

Suggested-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Hans van Kranenburg <hans@knorrie.org>
Fixes: e006b2e3be ("libxl: fix libacpi dependency")

give meaningful error message if qemu device model is unavailable

There's no sense to switch to qemu-xen-traditional device model
if that one is not enabled in the first place. This way we'll
have a chance later to print a message suggesting to install the
missing qemu package if we *actually* need qemu for the device model.

xen/arch/x86: make objdump output user locale agnostic

The objdump output is fed to grep, so make sure it doesn't change with
different user locales and break the grep parsing.
This problem was identified while updating xen in Debian and the fix is
needed for generating reproducible builds in varying environments.

Signed-off-by: Maximilian Engelhardt <maxi@daemonizer.de>

docs: set date to SOURCE_DATE_EPOCH if available

Use the solution described in [1] to replace the call to the 'date'
command with a version that uses SOURCE_DATE_EPOCH if available. This
is needed for reproducible builds.

[1] https://reproducible-builds.org/docs/source-date-epoch/

Signed-off-by: Maximilian Engelhardt <maxi@daemonizer.de>
[Hans van Kranenburg]
Note: this patch is submitted upstream but not committed yet. We
expect that it gets in. Otherwise, we don't wait and already have it
here because I want to have the reproducible build work completed.

tools: don't build/ship xenmon

This is something that hasn't been touched (except for making it Python
3 compatible, which failed) since 2007. Don't build or ship it.

    -# xenmon
      File "/usr/sbin/xenmon", line 680
stop_cmd = "/usr/bin/pkill -INT -z global xenbaked"
    TabError: inconsistent use of tabs and spaces in indentation

Signed-off-by: Hans van Kranenburg <hans@knorrie.org>

tools/xl/bash-completion: also complete 'xen'

We have the `xen` alias for xl in Debian, since in the past it was a
command that could execute either xl or xm.

Now, it always does xl, so, complete the same stuff for it as we have
for xl.

Signed-off-by: Hans van Kranenburg <hans@knorrie.org>
[git-debrebase split: mixed commit: upstream part]

pygrub: Specify -rpath LIBEXEC_LIB when building fsimage.so

If LIBEXEC_LIB is not on the default linker search path, the python
fsimage.so module fails to find libfsimage.so.

Add the relevant directory to the rpath explicitly.

(This situation occurs in the Debian package, where
--with-libexec-libdir is used to put each Xen version's libraries and
utilities in their own directory, to allow them to be coinstalled.)

Signed-off-by: Ian Jackson <ian.jackson@citrix.com>

pygrub: Set sys.path

We install libfsimage in a non-standard path for Reasons.
(See debian/rules.)

This patch was originally part of `tools-pygrub-prefix.diff'
(eg commit 51657319be54) and included changes to the Makefile to
change the installation arrangements (we do that part in the rules now
since that is a lot less prone to conflicts when we update) and to
shared library rpath (which is now done in a separate patch).

(Commit message rewritten by Ian Jackson.)

Signed-off-by: Ian Jackson <ian.jackson@citrix.com>
squash! pygrub: Set sys.path and rpath

hotplug-common: Do not adjust LD_LIBRARY_PATH

This is in the upstream script because on non-Debian systems, the
default install locations in /usr/local/lib might not be on the linker
path, and as a result the hotplug scripts would break.

A reason we might need it in Debian is our multiple version
coinstallation scheme.  However, the hotplug scripts all call the
utilities via the wrappers, and the binaries are configured to load
from the right place anyway.

This setting is an annoyance because it requires libdir, which is an
arch-specific path but comes from a file we want to put in
xen-utils-common, an arch:all package.

So drop this setting.

Signed-off-by: Ian Jackson <ian.jackson@citrix.com>

sysconfig.xencommons.in: Strip and debianize

Strip all options that are for stuff we don't ship, which is 1)
xenstored as stubdom and 2) the new options for oom score and open file
descriptor limit, which would not have any effect, because we're
shipping different init scripts... :|

It seems useful to give the user the option to revert to xenstored
instead of the default oxenstored if they really want.

Signed-off-by: Hans van Kranenburg <hans@knorrie.org>
Acked-by: Ian Jackson <ijackson@chiark.greenend.org.uk>

t/h/L/vif-common.sh: disable handle_iptable

Also see Debian bug #894013. The current attempt at providing
anti-spoofing rules results in a situation that does not have any
effect. Also note that forwarding bridged traffic to iptables is not
enabled by default, and that for openvswitch users it does not make any
sense.

So, stop cluttering the live iptables ruleset.

This functionality seems to be introduced before 2004 and since then it
has never got some additional love.

It would be nice to have a proper discussion upstream about how Xen
could provide some anti mac/ip spoofing in the dom0. It does not seem to
be a trivial thing to do, since it requires having quite some knowledge
about what the domU is allowed to do or not (e.g. a domU can be a
router...).

Signed-off-by: Hans van Kranenburg <hans@knorrie.org>

docs/man/xen-vbd-interface.7: Provide properly-formatted NAME section

This manpage was omitted from
   docs/man: Provide properly-formatted NAME sections
because I was previously building with markdown not installed.

Signed-off-by: Ian Jackson <ian.jackson@citrix.com>

shim: Provide separate install-shim target

When building on a 32-bit userland, the user wants to build 32-bit
tools and a 64-bit hypervisor.  This involves setting XEN_TARGET_ARCH
to different values for the tools build and the hypervisor build.

So the user must invoke the tools build and the hypervisor build
separately.

However, although the shim is done by the tools/firmware Makefile, its
bitness needs to be the same as the hypervisor, not the same as the
tools.  When run with XEN_TARGET_ARCH=x86_32, it it skipped, which is
wrong.

So the user must invoke the shim build separately.  This can be done
with
   make -C tools/firmware/xen-dir XEN_TARGET_ARCH=x86_64

However, tools/firmware/xen-dir has no `install' target.  The
installation of all `firmware' is done in tools/firmware/Makefile.  It
might be possible to fix this, but it is not trivial.  For example,
the definitions of INST_DIR and DEBG_DIR would need to be copied, as
would an appropriate $(INSTALL_DIR) call.

For now, provide an `install-shim' target in tools/firmware/Makefile.

This has to be called from `install' of course.  We can't make it
a dependency of `install' because it might be run before `all' has
completed.  We could make it depend on a `shim' target but such
a target is nearly impossible to write because everything is done by
the inflexible subdir-$@ machinery.

The overally result of this patch is that existing make invocations
work as before.  But additionally, the user can say
  make -C tools/firmware install-shim XEN_TARGET_ARCH=x86_64
to install the shim.  The user must have built it already.
Unlike the build rune, this install-rune is properly conditional
so it is OK to call on ARM.

What a mess.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>

config/Tools.mk.in: Respect caller's CONFIG_PV_SHIM

This makes it easier to disable the shim build.  (In Debian we need to
build the shim separately because it needs different compiler flags).

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
[ Hans: adjust from tools/firmware/Makefile to config/Tools.mk.in to
follow changes that happened in 8845155c83 ("pvshim: make PV shim build
selectable from configure") ]
Signed-off-by: Hans van Kranenburg <hans@knorrie.org>

.gitignore: Add configure output which we always delete and regenerate

Signed-off-by: Ian Jackson <ian.jackson@citrix.com>

autoconf: Provide libexec_libdir_suffix

This is going to be used to put libfsimage.so into a path containing
the multiarch triplet.

Signed-off-by: Ian Jackson <ian.jackson@citrix.com>

tools-libfsimage-prefix.diff

\o/

Do not build the instruction emulator

Signed-off-by: Ian Jackson <ian.jackson@citrix.com>

Remove static solaris support from pygrub

Patch-Name: tools-pygrub-remove-static-solaris-support

Gbp-Pq: Topic misc
Gbp-Pq: Name tools-pygrub-remove-static-solaris-support

Do not ship COPYING into /usr/include

This is not wanted in Debian.  COPYING ends up in
/usr/share/doc/xen-*copyright.

Patch-Name: tools-include-no-COPYING.diff

Signed-off-by: Ian Jackson <ian.jackson@citrix.com>

config-prefix.diff

Patch-Name: config-prefix.diff

Gbp-Pq: Topic prefix-abiname
Gbp-Pq: Name config-prefix.diff

Display Debian package version in hypervisor log

During hypervisor boot, disable the banner and nicely display the xen
version as well as the Maintainer address from debian/control.

For this to work the SOURCE_BASE_DIR variable needs to be set by the
build system to the top directory, i.e. where the debian folder is.

Original patch by Bastian Blank <waldi@debian.org>
Modified by
Hans van Kranenburg <hans@knorrie.org>
Maximilian Engelhardt <maxi@daemonizer.de>

Delete configure output

These autogenerated files are not useful in Debian; dh_autoreconf will
regenerate them.

If this patch does not apply when rebasing, you can simply delete the
files again.

Signed-off-by: Ian Jackson <ian.jackson@citrix.com>

Delete config.sub and config.guess

dh_autoreconf will provide these back.

If this patch does not apply when rebasing, you can simply delete the
files again.

Signed-off-by: Ian Jackson <ian.jackson@citrix.com>

Update changelog for new upstream 4.16.2+90-g0d39a6d1ae

[git-debrebase changelog: new upstream 4.16.2+90-g0d39a6d1ae]

Update to upstream 4.16.2+90-g0d39a6d1ae

[git-debrebase anchor: new upstream 4.16.2+90-g0d39a6d1ae, merge]

CHANGELOG: update link for RELEASE-4.16.0

Signed-off-by: Henry Wang <Henry.Wang@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>

xen/sched: migrate timers to correct cpus after suspend

Today all timers are migrated to cpu 0 when the system is being
suspended. They are not migrated back after resuming the system again.

This results (at least) to visible problems with the credit scheduler,
as the timer isn't handled on the cpu it was expected to occur, which
will result in an ASSERT() triggering. Other more subtle problems, like
uninterrupted elongated time slices, are probable. The least effect
will be worse performance on cpu 0 resulting from most scheduling
related timer interrupts happening there after suspend/resume.

Add migrating the scheduling related timers of a specific cpu from cpu
0 back to its original cpu when that cpu has gone up when resuming the
system.

Fixes: 0763cd268789 ("xen/sched: don't disable scheduler on cpus during suspend")
Signed-off-by: Juergen Gross <jgross@suse.com>
Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Dario Faggioli <dfaggioli@suse.com>
master commit: 37f82facd62f720fdcec104f72f86b8c6c214820
master date: 2022-11-04 09:03:23 +0100

tools/xenstore: call remove_domid_from_perm() for special nodes

When destroying a domain, any stale permissions of the domain must be
removed from the special nodes "@...", too. This was not done in the
fix for XSA-322.

Fixes: 496306324d8d ("tools/xenstore: revoke access rights for removed domains")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
master commit: 0751a75e3996cf6efd3925a90b4776660d8df2bc
master date: 2022-11-02 12:08:22 +0100

x86/spec-ctrl: Mitigate IBPB not flushing the RSB/RAS

Introduce spec_ctrl_new_guest_context() to encapsulate all logic pertaining to
using MSR_PRED_CMD for a new guest context, even if it only has one user
presently.

Introduce X86_BUG_IBPB_NO_RET, and use it extend spec_ctrl_new_guest_context()
with a manual fixup for hardware which mis-implements IBPB.

This is part of XSA-422 / CVE-2022-23824.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 2b27967fb89d7904a1571a2fb963b1c9cac548db)

x86/spec-ctrl: Enumeration for IBPB_RET

The IBPB_RET bit indicates that the CPU's implementation of MSR_PRED_CMD.IBPB
does flush the RSB/RAS too.

This is part of XSA-422 / CVE-2022-23824.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 24496558e650535bdbd22cc04731e82276cd1b3f)

tools/xenstore: harden transaction finalization against errors

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 2dd823ca7237e7fb90c890642d6a3b357a26fcff)

tools/xenstore: fix deleting node in transaction

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 13ac37f1416cae88d97f7baf6cf2a827edb9a187)

tools/ocaml: Ensure packet size is never negative

Integers in Ocaml have 63 or 31 bits of signed precision.

On 64-bit builds of Ocaml, this is fine because a C uint32_t always fits
within a 63-bit signed integer.

In 32-bit builds of Ocaml, this goes wrong.  The C uint32_t is truncated
first (loses the top bit), then has a unsigned/signed mismatch.

A "negative" value (i.e. a packet on the ring of between 1G and 2G in size)
will trigger an exception later in Bytes.make in xb.ml, and because the packet
is not removed from the ring, the exception re-triggers on every subsequent
query, creating a livelock.

Fix both the source of the exception in Xb, and as defence in depth, mark the
domain as bad for any Invalid_argument exceptions to avoid the risk of
livelock.

This is XSA-420 / CVE-2022-42324.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit ae34df4d82636f4c82700b447ea2c93b9f82b3f3)

tools/ocaml/xenstored: Fix quota bypass on domain shutdown

XSA-322 fixed a domid reuse vulnerability by assigning Dom0 as the owner of
any nodes left after a domain is shutdown (e.g. outside its /local/domain/N
tree).

However Dom0 has no quota on purpose, so this opened up another potential
attack vector. Avoid it by deleting these nodes instead of assigning them to
Dom0.

This is part of XSA-419 / CVE-2022-42323.

Fixes: c46eff921209 ("tools/ocaml/xenstored: clean up permissions for dead domains")
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit db471408edd46af403b8bd44d180a928ad7fbb80)

docs: enhance xenstore.txt with permissions description

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 d084d2c6dff7044956ebdf83a259ad6081a1d921)

tools/xenstore: make the internal memory data base the default

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 d174fefa90487ddd25ebc618028f67b2e8a1f795)

tools/xenstore: remove nodes owned by destroyed domain

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: 496306324d8d ("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 755d3f9debf8879448211fffb018f556136f6a79)

tools/xenstore: use treewalk for creating node records

Instead of doing an open tree walk using call recursion, use
walk_node_tree() when creating the node records during a live update.

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 297ac246a5d8ed656b349641288f3402dcc0251e)

tools/xenstore: use treewalk for deleting nodes

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 ea16962053a6849a6e7cada549ba7f8c586d85c6)

tools/xenstore: use treewalk for check_store()

Instead of doing an open tree walk using call recursion, use
walk_node_tree() when checking the store for inconsistencies.

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

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

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit a07cc0ec60612f414bedf2bafb26ec38d2602e95)

tools/xenstore: simplify check_store()

check_store() is using a hash table for storing all node names it has
found via walking the tree. Additionally it using another hash table
for all children of a node to detect duplicate child names.

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

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

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 70f719f52a220bc5bc987e4dd28e14a7039a176b)

tools/xenstore: add generic treewalk function

Add a generic function to walk the complete node tree. It will start
at "/" and descend recursively into each child, calling a function
specified by the caller. Depending on the return value of the user
specified function the walk will be aborted, continued, or the current
child will be skipped by not descending into its children.

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

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 0d7c5d19bc27492360196e7dad2b227908564fff)

tools/xenstore: don't let remove_child_entry() call corrupt()

In case of write_node() returning an error, remove_child_entry() will
call corrupt() today. This could result in an endless recursion, as
remove_child_entry() is called by corrupt(), too:

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

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

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

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 0c00c51f3bc8206c7f9cf87d014650157bee2bf4)

tools/xenstore: remove recursion from construct_node()

In order to reduce stack usage due to recursion, switch
construct_node() to use a loop instead.

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

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit da8ee25d02a5447ba39a9800ee2a710ae1f54222)

tools/xenstore: fix checking node permissions

Today chk_domain_generation() is being used to check whether a node
permission entry is still valid or whether it is referring to a domain
no longer existing. This is done by comparing the node's and the
domain's generation count.

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

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

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

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

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit ab128218225d3542596ca3a02aee80d55494bef8)

tools/xenstore: don't use conn->in as context for temporary allocations

Using the struct buffered data pointer of the current processed request
for temporary data allocations has a major drawback: the used area (and
with that the temporary data) is freed only after the response of the
request has been written to the ring page or has been read via the
socket. This can happen much later in case a guest isn't reading its
responses fast enough.

As the temporary data can be safely freed after creating the response,
add a temporary context for that purpose and use that for allocating
the temporary memory, as it was already the case before commit
cc0612464896 ("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: cc0612464896 ("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 2a587de219cc0765330fbf9fac6827bfaf29e29b)

SUPPORT.md: clarify support of untrusted driver domains with oxenstored

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 c7bc20d8d123851a468402bbfc9e3330efff21ec)

tools/ocaml: Limit maximum in-flight requests / outstanding replies

Introduce a limit on the number of outstanding reply packets in the xenbus
queue.  This limits the number of in-flight requests: when the output queue is
full we'll stop processing inputs until the output queue has room again.

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit 9284ae0c40fb5b9606947eaaec23dc71d0540e96)

tools/ocaml/xb: Add BoundedQueue

Ensures we cannot store more than [capacity] elements in a [Queue].  Replacing
all Queue with this module will then ensure at compile time that all Queues
are correctly bound checked.

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

No functional change.

This is part of XSA-326.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit 19171fb5d888b4467a7073e8febc5e05540956e9)

tools/ocaml: Change Xb.input to return Packet.t option

The queue here would only ever hold at most one element.  This will simplify
follow-up patches.

This is part of XSA-326.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit c0a86a462721008eca5ff733660de094d3c34bc7)

tools/ocaml/libs/xb: hide type of Xb.t

Hiding the type will make it easier to change the implementation
in the future without breaking code that relies on it.

No functional change.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit 7ade30a1451734d041363c750a65d322e25b47ba)

tools/ocaml: GC parameter tuning

By default the OCaml garbage collector would return memory to the OS only
after unused memory is 5x live memory.  Tweak this to 120% instead, which
would match the major GC speed.

This is part of XSA-326.

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit 4a8bacff20b857ca0d628ef5525877ade11f2a42)

tools/ocaml/xenstored: Check for maxrequests before performing operations

Previously we'd perform the operation, record the updated tree in the
transaction record, then try to insert a watchop path and the reply packet.

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

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

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

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit 329f4d1a6535c6c5a34025ca0d03fc5c7228fcff)

tools/ocaml/xenstored: Synchronise defaults with oxenstore.conf.in

We currently have 2 different set of defaults in upstream Xen git tree:
* defined in the source code, only used if there is no config file
* defined in the oxenstored.conf.in upstream Xen

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

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

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

Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Acked-by: Christian Lindig <christian.lindig@citrix.com>
(cherry picked from commit 84734955d4bf629ba459a74773afcde50a52236f)

tools/xenstore: add control command for setting and showing quota

Add a xenstore-control command "quota" to:
- show current quota settings
- change quota settings
- show current quota related values of a domain

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

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 9c484bef83496b683b0087e3bd2a560da4aa37af)

tools/xenstore: add exports for quota variables

Some quota variables are not exported via header files.

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 1da16d5990b5f7752657fca3e948f735177ea9ad)

tools/xenstore: add memory accounting for nodes

Add the memory accounting for Xenstore nodes. In order to make this
not too complicated allow for some sloppiness when writing nodes. Any
hard quota violation will result in no further requests to be accepted.

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

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 00e9e32d022be1afc144b75acdaeba8393e63315)

tools/xenstore: add memory accounting for watches

Add the memory accounting for registered watches.

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

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

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 7f9978a2cc37aaffab2fb09593bc598c0712a69b)

tools/xenstore: add memory accounting for responses

Add the memory accounting for queued responses.

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

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

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit f6d00133643a524d2138c9e3f192bbde719050ba)

tools/xenstore: add infrastructure to keep track of per domain memory usage

The amount of memory a domain can consume in Xenstore is limited by
various quota today, but even with sane quota a domain can still
consume rather large memory quantities.

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

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

Setting any of those quotas to 0 will disable it.

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

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 0d4a8ec7a93faedbe54fd197db146de628459e77)

tools/xenstore: move the call of setup_structure() to dom0 introduction

Setting up the basic structure when introducing dom0 has the advantage
to be able to add proper node memory accounting for the added nodes
later.

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

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

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 60e2f6020dea7f616857b8fc1141b1c085d88761)

tools/xenstore: limit max number of nodes accessed in a transaction

Today a guest is free to access as many nodes in a single transaction
as it wants. This can lead to unbounded memory consumption in Xenstore
as there is the need to keep track of all nodes having been accessed
during a transaction.

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

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

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

Suggested-by: Julien Grall <julien@xen.org>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 268369d8e322d227a74a899009c5748d7b0ea142)

tools/xenstore: simplify and fix per domain node accounting

The accounting of nodes can be simplified now that each connection
holds the associated domid.

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

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

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

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit dbef1f7482894c572d90cd73d99ed689c891e863)

tools/xenstore: fix connection->id usage

Don't use conn->id for privilege checks, but domain_is_unprivileged().

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 3047df38e1991510bc295e3e1bb6b6b6c4a97831)

tools/xenstore: don't buffer multiple identical watch events

A guest not reading its Xenstore response buffer fast enough might
pile up lots of Xenstore watch events buffered. Reduce the generated
load by dropping new events which already have an identical copy
pending.

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

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit b5c0bdb96d33e18c324c13d8e33c08732d77eaa2)

tools/xenstore: limit outstanding requests

Add another quota for limiting the number of outstanding requests of a
guest. As the way to specify quotas on the command line is becoming
rather nasty, switch to a new scheme using [--quota|-Q] <what>=<val>
allowing to add more quotas in future easily.

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

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

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

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

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

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 36de433a273f55d614c83b89c9a8972287a1e475)

tools/xenstore: let unread watch events time out

A future modification will limit the number of outstanding requests
for a domain, where "outstanding" means that the response of the
request or any resulting watch event hasn't been consumed yet.

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

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

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

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

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 5285dcb1a5c01695c11e6397c95d906b5e765c98)

tools/xenstore: reduce number of watch events

When removing a watched node outside of a transaction, two watch events
are being produced instead of just a single one.

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

This happens e.g.:

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

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

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

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 3a96013a3e17baa07410b1b9776225d1d9a74297)

tools/xenstore: add helpers to free struct buffered_data

Add two helpers for freeing struct buffered_data: free_buffered_data()
for freeing one instance and conn_free_buffered_data() for freeing all
instances for a connection.

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

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit ead062a68a9c201a95488e84750a70a107f7b317)

tools/xenstore: split up send_reply()

Today send_reply() is used for both, normal request replies and watch
events.

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

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

This is part of XSA-326.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 9bfde319dbac2a1321898d2f75a3f075c3eb7b32)

tools/xenstore: Fail a transaction if it is not possible to create a node

Commit f2bebf72c4d5 "xenstore: rework of transaction handling" moved
out from copying the entire database everytime a new transaction is
opened to track the list of nodes changed.

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

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

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

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

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

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

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

Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
(cherry picked from commit 5d71766bd1a4a3a8b2fe952ca2be80e02fe48f34)

tools/xenstore: create_node: Don't defer work to undo any changes on failure

XSA-115 extended destroy_node() to update the node accounting for the
connection. The implementation is assuming the connection is the parent
of the node, however all the nodes are allocated using a separate context
(see process_message()). This will result to crash (or corrupt) xenstored
as the pointer is wrongly used.

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

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

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

Fixes: 0bfb2101f243 ("tools/xenstore: fix node accounting after failed node creation")
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
(cherry picked from commit 1cd3cc7ea27cda7640a8d895e09617b61c265697)

x86/vmx: Revert "VMX: use a single, global APIC access page"

The claim "No accesses would ever go to this page." is false.  A consequence
of how Intel's APIC Acceleration works, and Xen's choice to have per-domain
P2Ms (rather than per-vCPU P2Ms) means that the APIC page is fully read-write
to any vCPU which is not in xAPIC mode.

This reverts commit 58850b9074d3e7affdf3bc94c84e417ecfa4d165.

This is XSA-412 / CVE-2022-42327.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 3b5beaf49033cddf4b2cc4e4d391b966f4203471)

x86/pv-shim: correct ballooning down for compat guests

The compat layer for multi-extent memory ops may need to split incoming
requests. Since the guest handles in the interface structures may not be
altered, it does so by leveraging do_memory_op()'s continuation
handling: It hands on non-initial requests with a non-zero start extent,
with the (native) handle suitably adjusted down. As a result
do_memory_op() sees only the first of potentially several requests with
start extent being zero. In order to be usable as overall result, the
function accumulates args.nr_done, i.e. it initialized the field with
the start extent. Therefore non-initial requests resulting from the
split would pass too large a number into pv_shim_offline_memory().

Address that breakage by always calling pv_shim_offline_memory()
regardless of current hypercall preemption status, with a suitably
adjusted first argument. Note that this is correct also for the native
guest case: We now simply "commit" what was completed right away, rather
than at the end of a series of preemption/re-start cycles. In fact this
improves overall preemption behavior: There's no longer a potentially
big chunk of work done non-preemptively at the end of the last
"iteration".

Fixes: b2245acc60c3 ("xen/pvshim: memory hotplug")
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 1d7fbc535d1d37bdc2cc53ede360b0f6651f7de1
master date: 2022-10-28 15:49:33 +0200

x86/pv-shim: correct ballooning up for compat guests

The compat layer for multi-extent memory ops may need to split incoming
requests. Since the guest handles in the interface structures may not be
altered, it does so by leveraging do_memory_op()'s continuation
handling: It hands on non-initial requests with a non-zero start extent,
with the (native) handle suitably adjusted down. As a result
do_memory_op() sees only the first of potentially several requests with
start extent being zero. It's only that case when the function would
issue a call to pv_shim_online_memory(), yet the range then covers only
the first sub-range that results from the split.

Address that breakage by making a complementary call to
pv_shim_online_memory() in compat layer.

Fixes: b2245acc60c3 ("xen/pvshim: memory hotplug")
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: a0bfdd201ea12aa5679bb8944d63a4e0d3c23160
master date: 2022-10-28 15:48:50 +0200

x86/pv-shim: correctly ignore empty onlining requests

Mem-op requests may have zero extents. Such requests need treating as
no-ops. pv_shim_online_memory(), however, would have tried to take 2³²-1
order-sized pages from its balloon list (to then populate them),
typically ending when the entire set of ballooned pages of this order
was consumed.

Note that pv_shim_offline_memory() does not have such an issue.

Fixes: b2245acc60c3 ("xen/pvshim: memory hotplug")
Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 9272225ca72801fd9fa5b268a2d1c5adebd19cd9
master date: 2022-10-28 15:47:59 +0200

common: map_vcpu_info() wants to unshare the underlying page

Not passing P2M_UNSHARE to get_page_from_gfn() means there won't even be
an attempt to unshare the referenced page, without any indication to the
caller (e.g. -EAGAIN). Note that guests have no direct control over
which of their pages are shared (or paged out), and hence they have no
way to make sure all on their own that the subsequent obtaining of a
writable type reference can actually succeed.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
master commit: 48980cf24d5cf41fd644600f99c753419505e735
master date: 2022-10-28 11:38:32 +0200

x86: also zap secondary time area handles during soft reset

Just like domain_soft_reset() properly zaps runstate area handles, the
secondary time area ones also need discarding to prevent guest memory
corruption once the guest is re-started.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: b80d4f8d2ea6418e32fb4f20d1304ace6d6566e3
master date: 2022-10-27 11:49:09 +0200

vpci/msix: remove from table list on detach

Teardown of MSIX vPCI related data doesn't currently remove the MSIX
device data from the list of MSIX tables handled by the domain,
leading to a use-after-free of the data in the msix structure.

Remove the structure from the list before freeing in order to solve
it.

Reported-by: Jan Beulich <jbeulich@suse.com>
Fixes: d6281be9d0 ('vpci/msix: add MSI-X handlers')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: c14aea137eab29eb9c30bfad745a00c65ad21066
master date: 2022-10-26 14:56:58 +0200

vpci: don't assume that vpci per-device data exists unconditionally

It's possible for a device to be assigned to a domain but have no
vpci structure if vpci_process_pending() failed and called
vpci_remove_device() as a result.  The unconditional accesses done by
vpci_{read,write}() and vpci_remove_device() to pdev->vpci would
then trigger a NULL pointer dereference.

Add checks for pdev->vpci presence in the affected functions.

Fixes: 9c244fdef7 ('vpci: add header handlers')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 6ccb5e308ceeb895fbccd87a528a8bd24325aa39
master date: 2022-10-26 14:55:30 +0200

x86/shadow: drop (replace) bogus assertions

The addition of a call to shadow_blow_tables() from shadow_teardown()
has resulted in the "no vcpus" related assertion becoming triggerable:
If domain_create() fails with at least one page successfully allocated
in the course of shadow_enable(), or if domain_create() succeeds and
the domain is then killed without ever invoking XEN_DOMCTL_max_vcpus.
Note that in-tree tests (test-resource and test-tsx) do exactly the
latter of these two.

The assertion's comment was bogus anyway: Shadow mode has been getting
enabled before allocation of vCPU-s for quite some time. Convert the
assertion to a conditional: As long as there are no vCPU-s, there's
nothing to blow away.

Fixes: e7aa55c0aab3 ("x86/p2m: free the paging memory pool preemptively")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
A similar assertion/comment pair exists in _shadow_prealloc(); the
comment is similarly bogus, and the assertion could in principle trigger
e.g. when shadow_alloc_p2m_page() is called early enough. Replace those
at the same time by a similar early return, here indicating failure to
the caller (which will generally lead to the domain being crashed in
shadow_prealloc()).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: a92dc2bb30ba65ae25d2f417677eb7ef9a6a0fef
master date: 2022-10-24 15:46:11 +0200

xen/sched: fix restore_vcpu_affinity() by removing it

When the system is coming up after having been suspended,
restore_vcpu_affinity() is called for each domain in order to adjust
the vcpu's affinity settings in case a cpu didn't come to live again.

The way restore_vcpu_affinity() is doing that is wrong, because the
specific scheduler isn't being informed about a possible migration of
the vcpu to another cpu. Additionally the migration is often even
happening if all cpus are running again, as it is done without check
whether it is really needed.

As cpupool management is already calling cpu_disable_scheduler() for
cpus not having come up again, and cpu_disable_scheduler() is taking
care of eventually needed vcpu migration in the proper way, there is
simply no need for restore_vcpu_affinity().

So just remove restore_vcpu_affinity() completely, together with the
no longer used sched_reset_affinity_broken().

Fixes: 8a04eaa8ea83 ("xen/sched: move some per-vcpu items to struct sched_unit")
Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Dario Faggioli <dfaggioli@suse.com>
Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
master commit: fce1f381f7388daaa3e96dbb0d67d7a3e4bb2d2d
master date: 2022-10-24 11:16:27 +0100

xen/sched: fix race in RTDS scheduler

When a domain gets paused the unit runnable state can change to "not
runnable" without the scheduling lock being involved. This means that
a specific scheduler isn't involved in this change of runnable state.

In the RTDS scheduler this can result in an inconsistency in case a
unit is losing its "runnable" capability while the RTDS scheduler's
scheduling function is active. RTDS will remove the unit from the run
queue, but doesn't do so for the replenish queue, leading to hitting
an ASSERT() in replq_insert() later when the domain is unpaused again.

Fix that by removing the unit from the replenish queue as well in this
case.

Fixes: 7c7b407e7772 ("xen/sched: introduce unit_runnable_state()")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Dario Faggioli <dfaggioli@suse.com>
master commit: 73c62927f64ecb48f27d06176befdf76b879f340
master date: 2022-10-21 12:32:23 +0200

EFI: don't convert memory marked for runtime use to ordinary RAM

efi_init_memory() in both relevant places is treating EFI_MEMORY_RUNTIME
higher priority than the type of the range. To avoid accessing memory at
runtime which was re-used for other purposes, make
efi_arch_process_memory_map() follow suit. While in theory the same would
apply to EfiACPIReclaimMemory, we don't actually "reclaim" or clobber
that memory (converted to E820_ACPI on x86) there (and it would be a bug
if the Dom0 kernel tried to reclaim the range, bypassing Xen's memory
management, plus it would be at least bogus if it clobbered that space),
hence that type's handling can be left alone.

Fixes: bf6501a62e80 ("x86-64: EFI boot code")
Fixes: facac0af87ef ("x86-64: EFI runtime code")
Fixes: 6d70ea10d49f ("Add ARM EFI boot support")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
master commit: f324300c8347b6aa6f9c0b18e0a90bbf44011a9a
master date: 2022-10-21 12:30:24 +0200

argo: Remove reachable ASSERT_UNREACHABLE

I observed this ASSERT_UNREACHABLE in partner_rings_remove consistently
trip.  It was in OpenXT with the viptables patch applied.

dom10 shuts down.
dom7 is REJECTED sending to dom10.
dom7 shuts down and this ASSERT trips for dom10.

The argo_send_info has a domid, but there is no refcount taken on
the domain.  Therefore it's not appropriate to ASSERT that the domain
can be looked up via domid.  Replace with a debug message.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Christopher Clark <christopher.w.clark@gmail.com>
master commit: 197f612b77c5afe04e60df2100a855370d720ad7
master date: 2022-10-14 14:45:41 +0100

VMX: correct error handling in vmx_create_vmcs()

With the addition of vmx_add_msr() calls to construct_vmcs() there are
now cases where simply freeing the VMCS isn't enough: The MSR bitmap
page as well as one of the MSR area ones (if it's the 2nd vmx_add_msr()
which fails) may also need freeing. Switch to using vmx_destroy_vmcs()
instead.

Fixes: 3bd36952dab6 ("x86/spec-ctrl: Introduce an option to control L1D_FLUSH for HVM HAP guests")
Fixes: 53a570b28569 ("x86/spec-ctrl: Support IBPB-on-entry")
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
master commit: 448d28309f1a966bdc850aff1a637e0b79a03e43
master date: 2022-10-12 17:57:56 +0200

x86emul: respect NSCB

protmode_load_seg() would better adhere to that "feature" of clearing
base (and limit) during NULL selector loads.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 87a20c98d9f0f422727fe9b4b9e22c2c43a5cd9c
master date: 2022-10-11 14:30:41 +0200

xen/arm: p2m: Populate pages for GICv2 mapping in p2m_init()

Hardware using GICv2 needs to create a P2M mapping of 8KB GICv2 area
when the domain is created. Considering the worst case of page tables
which requires 6 P2M pages as the two pages will be consecutive but not
necessarily in the same L3 page table and keep a buffer, populate 16
pages as the default value to the P2M pages pool in p2m_init() at the
domain creation stage to satisfy the GICv2 requirement. For GICv3, the
above-mentioned P2M mapping is not necessary, but since the allocated
16 pages here would not be lost, hence populate these pages
unconditionally.

With the default 16 P2M pages populated, there would be a case that
failures would happen in the domain creation with P2M pages already in
use. To properly free the P2M for this case, firstly support the
optionally preemption of p2m_teardown(), then call p2m_teardown() and
p2m_set_allocation(d, 0, NULL) non-preemptively in p2m_final_teardown().
As non-preemptive p2m_teardown() should only return 0, use a
BUG_ON to confirm that.

Since p2m_final_teardown() is called either after
domain_relinquish_resources() where relinquish_p2m_mapping() has been
called, or from failure path of domain_create()/arch_domain_create()
where mappings that require p2m_put_l3_page() should never be created,
relinquish_p2m_mapping() is not added in p2m_final_teardown(), add
in-code comments to refer this.

Fixes: cbea5a1149ca ("xen/arm: Allocate and free P2M pages from the P2M pool")
Suggested-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Henry Wang <Henry.Wang@arm.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
(cherry picked from commit: c7cff1188802646eaa38e918e5738da0e84949be)

arm/p2m: Rework p2m_init()

p2m_init() is mostly trivial initialisation, but has two fallible operations
which are on either side of the backpointer trigger for teardown to take
actions.

p2m_free_vmid() is idempotent with a failed p2m_alloc_vmid(), so rearrange
p2m_init() to perform all trivial setup, then set the backpointer, then
perform all fallible setup.

This will simplify a future bugfix which needs to add a third fallible
operation.

No practical change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
(cherry picked from commit: 3783e583319fa1ce75e414d851f0fde191a14753)

x86/vpmu: Fix race-condition in vpmu_load

The vPMU code-bases attempts to perform an optimization on saving/reloading the
PMU context by keeping track of what vCPU ran on each pCPU. When a pCPU is
getting scheduled, checks if the previous vCPU isn't the current one. If so,
attempts a call to vpmu_save_force. Unfortunately if the previous vCPU is
already getting scheduled to run on another pCPU its state will be already
runnable, which results in an ASSERT failure.

Fix this by always performing a pmu context save in vpmu_save when called from
vpmu_switch_from, and do a vpmu_load when called from vpmu_switch_to.

While this presents a minimal overhead in case the same vCPU is getting
rescheduled on the same pCPU, the ASSERT failure is avoided and the code is a
lot easier to reason about.

Signed-off-by: Tamas K Lengyel <tamas.lengyel@intel.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
master commit: defa4e51d20a143bdd4395a075bf0933bb38a9a4
master date: 2022-09-30 09:53:49 +0200

x86: wire up VCPUOP_register_vcpu_time_memory_area for 32-bit guests

Forever sinced its introduction VCPUOP_register_vcpu_time_memory_area
was available only to native domains. Linux, for example, would attempt
to use it irrespective of guest bitness (including in its so called
PVHVM mode) as long as it finds XEN_PVCLOCK_TSC_STABLE_BIT set (which we
set only for clocksource=tsc, which in turn needs engaging via command
line option).

Fixes: a5d39947cb89 ("Allow guests to register secondary vcpu_time_info")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: b726541d94bd0a80b5864d17a2cd2e6d73a3fe0a
master date: 2022-09-29 14:47:45 +0200

xen/gnttab: fix gnttab_acquire_resource()

Commit 9dc46386d89d ("gnttab: work around "may be used uninitialized"
warning") was wrong, as vaddrs can legitimately be NULL in case
XENMEM_resource_grant_table_id_status was specified for a grant table
v1. This would result in crashes in debug builds due to
ASSERT_UNREACHABLE() triggering.

Check vaddrs only to be NULL in the rc == 0 case.

Expand the tests in tools/tests/resource to tickle this path, and verify that
using XENMEM_resource_grant_table_id_status on a v1 grant table fails.

Fixes: 9dc46386d89d ("gnttab: work around "may be used uninitialized" warning")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com> # xen
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 52daa6a8483e4fbd6757c9d1b791e23931791608
master date: 2022-09-09 16:28:38 +0100

tools/xenstore: minor fix of the migration stream doc

Drop mentioning the non-existent read-only socket in the migration
stream description document.

The related record field was removed in commit 8868a0e3f674 ("docs:
update the xenstore migration stream documentation).

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
master commit: ace1d2eff80d3d66c37ae765dae3e3cb5697e5a4
master date: 2022-09-08 09:25:58 +0200

Config.mk: correct PIE-related option(s) in EMBEDDED_EXTRA_CFLAGS

I haven't been able to find evidence of "-nopie" ever having been a
supported compiler option. The correct spelling is "-no-pie".
Furthermore like "-pie" this is an option which is solely passed to the
linker. The compiler only recognizes "-fpie" / "-fPIE" / "-fno-pie", and
it doesn't infer these options from "-pie" / "-no-pie".

Add the compiler recognized form, but for the possible case of the
variable also being used somewhere for linking keep the linker option as
well (with corrected spelling).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Build: Drop -no-pie from EMBEDDED_EXTRA_CFLAGS

This breaks all Clang builds, as demostrated by Gitlab CI.

Contrary to the description in ecd6b9759919, -no-pie is not even an option
passed to the linker.  GCC's actual behaviour is to inhibit the passing of
-pie to the linker, as well as selecting different cr0 artefacts to be linked.

EMBEDDED_EXTRA_CFLAGS is not used for $(CC)-doing-linking, and not liable to
gain such a usecase.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Stefano Stabellini <sstabellini@kernel.org>
Fixes: ecd6b9759919 ("Config.mk: correct PIE-related option(s) in EMBEDDED_EXTRA_CFLAGS")
master commit: ecd6b9759919fa6335b0be1b5fc5cce29a30c4f1
master date: 2022-09-08 09:25:26 +0200
master commit: 13a7c0074ac8fb31f6c0485429b7a20a1946cb22
master date: 2022-09-27 15:40:42 -0700

xen/sched: fix cpu hotplug

Cpu unplugging is calling schedule_cpu_rm() via stop_machine_run() with
interrupts disabled, thus any memory allocation or freeing must be
avoided.

Since commit 5047cd1d5dea ("xen/common: Use enhanced
ASSERT_ALLOC_CONTEXT in xmalloc()") this restriction is being enforced
via an assertion, which will now fail.

Fix this by allocating needed memory before entering stop_machine_run()
and freeing any memory only after having finished stop_machine_run().

Fixes: 1ec410112cdd ("xen/sched: support differing granularity in schedule_cpu_[add/rm]()")
Reported-by: Gao Ruifeng <ruifeng.gao@intel.com>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: d84473689611eed32fd90b27e614f28af767fa3f
master date: 2022-09-05 11:42:30 +0100