We are shortly going to need to context switch new bits in both the vcpu and
S3 paths. Introduce SCF_IST_MASK and SCF_DOM_MASK, and rework d->arch.verw
into d->arch.spec_ctrl_flags to accommodate.
No functional change.
This is part of XSA-407.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 5796912f72)
libxl__xs_directory() can potentially return NULL without setting `n`.
As `n` isn't initialised, we need to check libxl__xs_directory()
return value before checking `n`. Otherwise, `n` might be non-zero
with `bdfs` NULL which would lead to a segv.
Fixes: 57bff091f4 ("libxl: add 'name' field to 'libxl_device_pci' in the IDL...")
Reported-by: "G.R." <firemeteor@users.sourceforge.net>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Tested-by: "G.R." <firemeteor@users.sourceforge.net>
master commit: d778089ac7
master date: 2022-07-12 08:38:51 +0200
Missing prototype of asprintf() without _GNU_SOURCE.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Henry Wang <Henry.Wang@arm.com>
master commit: d693b22733
master date: 2022-07-12 08:38:35 +0200
Support controling the PV/HVM suboption of msr-sc/rsb/md-clear, which
previously wasn't possible.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 27357c394b
master date: 2022-07-11 15:21:35 +0100
This will help parsing a sub-option which has boolean and non-boolean options
available.
First, rework 'int val' into 'bool has_neg_prefix'. This inverts it's value,
but the resulting logic is far easier to follow.
Second, reject anything of the form 'no-$FOO=' which excludes ambiguous
constructs such as 'no-$foo=yes' which have never been valid.
This just leaves the case where everything is otherwise fine, but parse_bool()
can't interpret the provided string.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 382326cac5
master date: 2022-07-11 15:21:35 +0100
This was an oversight from when unpriv-mmio was introduced.
Fixes: 8c24b70fed ("x86/spec-ctrl: Add spec-ctrl=unpriv-mmio")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 4cdb519d79
master date: 2022-07-11 15:21:35 +0100
xc_psr.c:161:5: error: conflicting types for 'xc_psr_cmt_get_data'
due to enum/integer mismatch;
Signed-off-by: Charles Arnold <carnold@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
master commit: 8eeae8c2b4
master date: 2022-07-06 13:06:40 +0200
STIBP and PSFD are slightly weird bits, because they're both implied by other
bits in MSR_SPEC_CTRL. Add fine grain controls for them, and take the
implications into account when setting IBRS/SSBD.
Rearrange the IBPB text/variables/logic to keep all the MSR_SPEC_CTRL bits
together, for consistency.
However, AMD have a hardware hint CPUID bit recommending that STIBP be set
unilaterally. This is advertised on Zen3, so follow the recommendation.
Furthermore, in such cases, set STIBP behind the guest's back for now. This
has negligible overhead for the guest, but saves a WRMSR on vmentry. This is
the only default change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: fef244b179
master date: 2022-06-30 18:07:13 +0100
Back at the time of the original Spectre-v2 fixes, it was recommended to clear
MSR_SPEC_CTRL when going idle. This is because of the side effects on the
sibling thread caused by the microcode IBRS and STIBP implementations which
were retrofitted to existing CPUs.
However, there are no relevant cross-thread impacts for the hardware
IBRS/STIBP implementations, so this logic should not be used on Intel CPUs
supporting eIBRS, or any AMD CPUs; doing so only adds unnecessary latency to
the idle path.
Furthermore, there's no point playing with MSR_SPEC_CTRL in the idle paths if
SMT is disabled for other reasons.
Fixes: 8d03080d2a ("x86/spec-ctrl: Cease using thunk=lfence on AMD")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: ffc7694e0c
master date: 2022-06-30 18:07:13 +0100
At the moment, corrupt() is neither checking for allocation failure
nor freeing the allocated memory.
Harden the code by printing ENOMEM if the allocation failed and
free 'str' after the last use.
This is not considered to be a security issue because corrupt() should
only be called when Xenstored thinks the database is corrupted. Note
that the trigger (i.e. a guest reliably provoking the call) would be
a security issue.
Fixes: 06d17943f0 ("Added a basic integrity checker, and some basic ability to recover from store")
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
master commit: db3382dd4f
master date: 2022-06-23 13:44:10 +0100
The ehci<n> number was parsed but ignored.
Fixes: 322ecbe4ac ("console: add EHCI debug port based serial console")
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: d6d0cb659f
master date: 2022-06-16 14:23:37 +0100
As per [1] the expansion of the pirq_dpci() macro causes a -Waddress
controlled warning (enabled implicitly in our builds, if not by default)
tying the middle part of the involved conditional expression to the
surrounding boolean context. Work around this by introducing a local
inline function in the affected source file.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102967
master commit: 80ad8db8a4
master date: 2022-06-15 10:19:32 +0200
Exporting a variable with a dash doesn't work reliably, they may be
striped from the environment when calling a sub-make or sub-shell.
CFLAGS-stack-boundary start to be removed from env in patch "build:
set ALL_OBJS in main Makefile; move prelink.o to main Makefile" when
running `make "ALL_OBJS=.."` due to the addition of the quote. At
least in my empirical tests.
Fixes: 2740d96efd ("xen/build: have the root Makefile generates the CFLAGS")
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
master commit: aa390d513a
master date: 2022-01-28 11:44:33 +0100
Per Xen's support statement, PCI passthrough should be to trusted domains
because the overall system security depends on factors outside of Xen's
control.
As such, Xen, in a supported configuration, is not vulnerable to DRPW/SBDR.
However, users who have risk assessed their configuration may be happy with
the risk of DoS, but unhappy with the risk of cross-domain data leakage. Such
users should enable this option.
On CPUs vulnerable to MDS, the existing mitigations are the best we can do to
mitigate MMIO cross-domain data leakage.
On CPUs fixed to MDS but vulnerable MMIO stale data leakage, this option:
* On CPUs susceptible to FBSDP, mitigates cross-domain fill buffer leakage
using FB_CLEAR.
* On CPUs susceptible to SBDR, mitigates RNG data recovery by engaging the
srb-lock, previously used to mitigate SRBDS.
Both mitigations require microcode from IPU 2022.1, May 2022.
This is part of XSA-404.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
(cherry picked from commit 8c24b70fed)
The three *_NO bits indicate non-susceptibility to the SSDP, FBSDP and PSDP
data movement primitives.
FB_CLEAR indicates that the VERW instruction has re-gained it's Fill Buffer
flushing side effect. This is only enumerated on parts where VERW had
previously lost it's flushing side effect due to the MDS/TAA vulnerabilities
being fixed in hardware.
FB_CLEAR_CTRL is available on a subset of FB_CLEAR parts where the Fill Buffer
clearing side effect of VERW can be turned off for performance reasons.
This is part of XSA-404.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
(cherry picked from commit 2ebe8fe9b7)
Currently, VERW flushing to mitigate MDS is boot time conditional per domain
type. However, to provide mitigations for DRPW (CVE-2022-21166), we need to
conditionally use VERW based on the trustworthiness of the guest, and the
devices passed through.
Remove the PV/HVM alternatives and instead issue a VERW on the return-to-guest
path depending on the SCF_verw bit in cpuinfo spec_ctrl_flags.
Introduce spec_ctrl_init_domain() and d->arch.verw to calculate the VERW
disposition at domain creation time, and context switch the SCF_verw bit.
For now, VERW flushing is used and controlled exactly as before, but later
patches will add per-domain cases too.
No change in behaviour.
This is part of XSA-404.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
(cherry picked from commit e06b95c1d4)
While PGT_pae_xen_l2 will be zapped once the type refcount of an L2 page
reaches zero, it'll be retained as long as the type refcount is non-
zero. Hence any checking against the requested type needs to either zap
the bit from the type or include it in the used mask.
Fixes: 9186e96b19 ("x86/pv: Clean up _get_page_type()")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: c2095ac76b
master date: 2022-06-10 10:21:06 +0200
There are legitimate uses of WC mappings of RAM, e.g. for DMA buffers with
devices that make non-coherent writes. The Linux sound subsystem makes
extensive use of this technique.
For such usecases, the guest's DMA buffer is mapped and consistently used as
WC, and Xen doesn't interact with the buffer.
However, a mischevious guest can use WC mappings to deliberately create
non-coherency between the cache and RAM, and use this to trick Xen into
validating a pagetable which isn't actually safe.
Allocate a new PGT_non_coherent to track the non-coherency of mappings. Set
it whenever a non-coherent writeable mapping is created. If the page is used
as anything other than PGT_writable_page, force a cache flush before
validation. Also force a cache flush before the page is returned to the heap.
This is CVE-2022-26364, part of XSA-402.
Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: c1c9cae3a9
master date: 2022-06-09 14:23:37 +0200
On pre-CLFLUSHOPT AMD CPUs, CLFLUSH is weakely ordered with everything,
including reads and writes to the address, and LFENCE/SFENCE instructions.
This creates a multitude of problematic corner cases, laid out in the manual.
Arrange to use MFENCE on both sides of the CLFLUSH to force proper ordering.
This is part of XSA-402.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 062868a5a8
master date: 2022-06-09 14:23:07 +0200
Subsequent changes will want a fully flushing version.
Use the new helper rather than opencoding it in flush_area_local(). This
resolves an outstanding issue where the conditional sfence is on the wrong
side of the clflushopt loop. clflushopt is ordered with respect to older
stores, not to younger stores.
Rename gnttab_cache_flush()'s helper to avoid colliding in name.
grant_table.c can see the prototype from cache.h so the build fails
otherwise.
This is part of XSA-402.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 9a67ffee33
master date: 2022-06-09 14:22:38 +0200
Changeset 55f97f49b7 ("x86: Change cache attributes of Xen 1:1 page mappings
in response to guest mapping requests") attempted to keep the cacheability
consistent between different mappings of the same page.
The reason wasn't described in the changelog, but it is understood to be in
regards to a concern over machine check exceptions, owing to errata when using
mixed cacheabilities. It did this primarily by updating Xen's mapping of the
page in the direct map when the guest mapped a page with reduced cacheability.
Unfortunately, the logic didn't actually prevent mixed cacheability from
occurring:
* A guest could map a page normally, and then map the same page with
different cacheability; nothing prevented this.
* The cacheability of the directmap was always latest-takes-precedence in
terms of guest requests.
* Grant-mapped frames with lesser cacheability didn't adjust the page's
cacheattr settings.
* The map_domain_page() function still unconditionally created WB mappings,
irrespective of the page's cacheattr settings.
Additionally, update_xen_mappings() had a bug where the alias calculation was
wrong for mfn's which were .init content, which should have been treated as
fully guest pages, not Xen pages.
Worse yet, the logic introduced a vulnerability whereby necessary
pagetable/segdesc adjustments made by Xen in the validation logic could become
non-coherent between the cache and main memory. The CPU could subsequently
operate on the stale value in the cache, rather than the safe value in main
memory.
The directmap contains primarily mappings of RAM. PAT/MTRR conflict
resolution is asymmetric, and generally for MTRR=WB ranges, PAT of lesser
cacheability resolves to being coherent. The special case is WC mappings,
which are non-coherent against MTRR=WB regions (except for fully-coherent
CPUs).
Xen must not have any WC cacheability in the directmap, to prevent Xen's
actions from creating non-coherency. (Guest actions creating non-coherency is
dealt with in subsequent patches.) As all memory types for MTRR=WB ranges
inter-operate coherently, so leave Xen's directmap mappings as WB.
Only PV guests with access to devices can use reduced-cacheability mappings to
begin with, and they're trusted not to mount DoSs against the system anyway.
Drop PGC_cacheattr_{base,mask} entirely, and the logic to manipulate them.
Shift the later PGC_* constants up, to gain 3 extra bits in the main reference
count. Retain the check in get_page_from_l1e() for special_pages() because a
guest has no business using reduced cacheability on these.
This reverts changeset 55f97f49b7
This is CVE-2022-26363, part of XSA-402.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
master commit: ae09597da3
master date: 2022-06-09 14:22:08 +0200
... rather than opencoding the PAT/PCD/PWT attributes in __PAGE_HYPERVISOR_*
constants. These are going to be needed by forthcoming logic.
No functional change.
This is part of XSA-402.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 1be8707c75
master date: 2022-06-09 14:21:38 +0200
_get_page_type() suffers from a race condition where it incorrectly assumes
that because 'x' was read and a subsequent a cmpxchg() succeeds, the type
cannot have changed in-between. Consider:
CPU A:
1. Creates an L2e referencing pg
`-> _get_page_type(pg, PGT_l1_page_table), sees count 0, type PGT_writable_page
2. Issues flush_tlb_mask()
CPU B:
3. Creates a writeable mapping of pg
`-> _get_page_type(pg, PGT_writable_page), count increases to 1
4. Writes into new mapping, creating a TLB entry for pg
5. Removes the writeable mapping of pg
`-> _put_page_type(pg), count goes back down to 0
CPU A:
7. Issues cmpxchg(), setting count 1, type PGT_l1_page_table
CPU B now has a writeable mapping to pg, which Xen believes is a pagetable and
suitably protected (i.e. read-only). The TLB flush in step 2 must be deferred
until after the guest is prohibited from creating new writeable mappings,
which is after step 7.
Defer all safety actions until after the cmpxchg() has successfully taken the
intended typeref, because that is what prevents concurrent users from using
the old type.
Also remove the early validation for writeable and shared pages. This removes
race conditions where one half of a parallel mapping attempt can return
successfully before:
* The IOMMU pagetables are in sync with the new page type
* Writeable mappings to shared pages have been torn down
This is part of XSA-401 / CVE-2022-26362.
Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
master commit: 8cc5036bc3
master date: 2022-06-09 14:21:04 +0200
Various fixes for clarity, ahead of making complicated changes.
* Split the overflow check out of the if/else chain for type handling, as
it's somewhat unrelated.
* Comment the main if/else chain to explain what is going on. Adjust one
ASSERT() and state the bit layout for validate-locked and partial states.
* Correct the comment about TLB flushing, as it's backwards. The problem
case is when writeable mappings are retained to a page becoming read-only,
as it allows the guest to bypass Xen's safety checks for updates.
* Reduce the scope of 'y'. It is an artefact of the cmpxchg loop and not
valid for use by subsequent logic. Switch to using ACCESS_ONCE() to treat
all reads as explicitly volatile. The only thing preventing the validated
wait-loop being infinite is the compiler barrier hidden in cpu_relax().
* Replace one page_get_owner(page) with the already-calculated 'd' already in
scope.
No functional change.
This is part of XSA-401 / CVE-2022-26362.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
master commit: 9186e96b19
master date: 2022-06-09 14:20:36 +0200
IOMMU code mapping / unmapping devices and interrupts will misbehave if
a wrong command line option declared a function "phantom" when there's a
real device at that position. Warn about this and adjust the specified
stride (in the worst case ignoring the option altogether).
Requested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: 444b555dc9
master date: 2022-05-20 12:20:35 +0200
Intel LPSS has INTERRUPT_LINE set to 0xff by default, that is declared
by the PCI Local Bus Specification Revision 3.0 (from 2004) as
"unknown"/"no connection". Fallback to poll mode in this case.
The 0xff handling is x86-specific, the surrounding code is guarded with
CONFIG_X86 anyway.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: 6a2ea1a237
master date: 2022-05-20 12:19:45 +0200
While for C files the compiler is supposed to arrange for emitting
respective information, for assembly sources we're responsible ourselves.
Present GNU ld master started warning about such, and hence 2.39 is
anticipated to have this warning.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
master commit: 62d22296a9
master date: 2022-05-18 11:18:45 +0200
We cannot really avoid such and we're also not really at risk because of
them, as we control page table permissions ourselves rather than relying
on a loader of some sort. Present GNU ld master started warning about
such, and hence 2.39 is anticipated to have this warning.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <jgrall@amazon.com>
master commit: 68f5aac012
master date: 2022-05-18 11:17:19 +0200
Xen provides hypercalls to shutdown (SCHEDOP_shutdown{,_code}) and
resume a domain (XEN_DOMCTL_resumedomain). They can be used for checkpoint
where the expectation is the domain should continue as nothing happened
afterwards.
hvmemul_do_io() and handle_pio() will act differently if the return
code of hvm_send_ioreq() (resp. hvmemul_do_pio_buffer()) is X86EMUL_RETRY.
In this case, the I/O state will be reset to STATE_IOREQ_NONE (i.e
no I/O is pending) and/or the PC will not be advanced.
If the shutdown request happens right after the I/O was sent to the
IOREQ, then emulation code will end up to re-execute the instruction
and therefore forward again the same I/O (at least when reading IO port).
This would be problem if the access has a side-effect. A dumb example,
is a device implementing a counter which is incremented by one for every
access. When running shutdown/resume in a loop, the value read by the
OS may not be the old value + 1.
Add an extra boolean in the structure hvm_vcpu_io to indicate whether
the I/O was suspended. This is then used in place of checking the domain
is shutting down in hvmemul_do_io() and handle_pio() as they should
act on suspend (i.e. vcpu_start_shutdown_deferral() returns false) rather
than shutdown.
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Paul Durrant <paul@xen.org>
master commit: b7e0d89788
master date: 2022-05-06 17:16:22 +0100
LLVM LD doesn't strip the quotes from the section names, and so the
resulting binary ends up with section names like:
[ 1] ".text" PROGBITS ffff82d040200000 00008000
000000000018cbc1 0000000000000000 AX 0 0 4096
This confuses some tools (like gdb) and prevents proper parsing of the
binary.
The issue has already been reported and is being fixed in LLD. In
order to workaround this issue and keep the GNU ld support define
different DECL_SECTION macros depending on the used ld
implementation.
Drop the quotes from the definitions of the debug sections in
DECL_DEBUG{2}, as those quotes are not required for GNU ld either.
Fixes: 6254920587 ('x86: quote section names when defining them in linker script')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 702c9a800e
master date: 2022-05-02 08:51:45 +0200
Detect GNU and LLVM ld implementations. This is required for further
patches that will introduce diverging behaviour depending on the
linker implementation in use.
Note that LLVM ld returns "compatible with GNU linkers" as part of the
version string, so be on the safe side and use '^' to only match at
the start of the line in case LLVM ever decides to change the text to
use "compatible with GNU ld" instead.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Michal Orzel <michal.orzel@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
master commit: c70c4b624f
master date: 2022-05-02 08:50:39 +0200
Windows Server 2019 Essentials will unconditionally attempt to read
P5_MC_ADDR MSR at boot and throw a BSOD if injected a #GP.
Fix this by mapping MSR_P5_MC_{ADDR,TYPE} to
MSR_IA32_MCi_{ADDR,STATUS}, as reported also done by hardware in Intel
SDM "Mapping of the Pentium Processor Machine-Check Errors to the
Machine-Check Architecture" section.
Reported-by: Steffen Einsle <einsle@phptrix.de>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: ce59e472b5
master date: 2022-05-02 08:49:12 +0200
While it is okay for IOMMU page tables to be set up for guests starting
in PoD mode, actual device assignment may only occur once all PoD
entries have been removed from the P2M. So far this was enforced only
for boot-time assignment, and only in the tool stack.
Also use the new function to replace p2m_pod_entry_count(): Its unlocked
access to p2m->pod.entry_count wasn't really okay (irrespective of the
result being stale by the time the caller gets to see it). Nor was the
use of that function in line with the immediately preceding comment: A
PoD guest isn't just one with a non-zero entry count, but also one with
a non-empty cache (e.g. prior to actually launching the guest).
To allow the tool stack to see a consistent snapshot of PoD state, move
the tail of XENMEM_{get,set}_pod_target handling into a function, adding
proper locking there.
In libxl take the liberty to use the new local variable r also for a
pre-existing call into libxc.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: ad4312d764
master date: 2022-05-02 08:48:02 +0200
Besides the reporter's issue of hitting a NULL deref when !CONFIG_GDBSX,
XEN_DOMCTL_test_assign_device can legitimately end up having NULL passed
here, when the domctl was passed DOMID_INVALID.
Fixes: 71e617a6b8 ("use is_iommu_enabled() where appropriate...")
Reported-by: Cheyenne Wills <cheyenne.wills@gmail.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Reviewed-by: Juergen Gross <jgross@suse.com>
master commit: fa4d84e6dd
master date: 2022-04-26 10:25:54 +0200
Today iommu_do_domctl() is being called from arch_do_domctl() in the
"default:" case of a switch statement. This has led already to crashes
due to unvalidated parameters.
Fix that by moving the call of iommu_do_domctl() to the main switch
statement of do_domctl().
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> # Arm
master commit: 9cd7e31b3f
master date: 2022-04-26 10:23:58 +0200
Setting errno to a negative value makes no sense.
Fixes: e78e8b9bb6 ("libxl: Add interface for querying hypervisor about PCI topology")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 2419a159fb
master date: 2022-04-22 20:39:34 +0100
Setting errno to a negative error value makes no sense.
Fixes: cb99a64029 ("libxc: arm: allow passing a device tree blob to the guest")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 438e96ab47
master date: 2022-04-22 20:39:34 +0100
The claimed reason for setting errno to -1 is wrong. On x86
xc_domain_pod_target() will set errno to a sane value in the error
case.
Fixes: ff1745d588 ("tools: libxl: do not set the PoD target on ARM")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: a0fb7e0e73
master date: 2022-04-22 20:39:34 +0100
Setting errno to a negative value makes no sense.
Fixes: 6b6500b3cb ("tools/libs/evtchn: Add support for restricting a handle")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 60245b71c1
master date: 2022-04-22 20:39:34 +0100
If the direct map is incorrectly modified with interrupts disabled,
the required TLB flushes are degraded to flushing the local CPU only.
This could lead to very hard to diagnose problems as different CPUs will
end up with different views of memory. Although, no such issues have yet
been identified.
Change the check in the flush_area() macro to look at system_state
instead. This defers the switch from local to all later in the boot
(see xen/arch/x86/setup.c:__start_xen()). This is fine because
additional PCPUs are not brought up until after the system state is
SYS_STATE_smp_boot.
Signed-off-by: David Vrabel <dvrabel@amazon.co.uk>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
x86/flushtlb: remove flush_area check on system state
Booting with Shadow Stacks leads to the following assert on a debug
hypervisor:
Assertion 'local_irq_is_enabled()' failed at arch/x86/smp.c:265
----[ Xen-4.17.0-10.24-d x86_64 debug=y Not tainted ]----
CPU: 0
RIP: e008:[<ffff82d040345300>] flush_area_mask+0x40/0x13e
[...]
Xen call trace:
[<ffff82d040345300>] R flush_area_mask+0x40/0x13e
[<ffff82d040338a40>] F modify_xen_mappings+0xc5/0x958
[<ffff82d0404474f9>] F arch/x86/alternative.c#_alternative_instructions+0xb7/0xb9
[<ffff82d0404476cc>] F alternative_branches+0xf/0x12
[<ffff82d04044e37d>] F __start_xen+0x1ef4/0x2776
[<ffff82d040203344>] F __high_start+0x94/0xa0
This is due to SYS_STATE_smp_boot being set before calling
alternative_branches(), and the flush in modify_xen_mappings() then
using flush_area_all() with interrupts disabled. Note that
alternative_branches() is called before APs are started, so the flush
must be a local one (and indeed the cpumask passed to
flush_area_mask() just contains one CPU).
Take the opportunity to simplify a bit the logic and make flush_area()
an alias of flush_area_all() in mm.c, taking into account that
cpu_online_map just contains the BSP before APs are started. This
requires widening the assert in flush_area_mask() to allow being
called with interrupts disabled as long as it's strictly a local only
flush.
The overall result is that a conditional can be removed from
flush_area().
While there also introduce an ASSERT to check that a vCPU state flush
is not issued for the local CPU only.
Fixes: 78e072bc37 ('x86/mm: avoid inadvertently degrading a TLB flush to local only')
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 78e072bc37
master date: 2022-04-20 10:55:01 +0200
master commit: 9f735ee490
master date: 2022-05-25 11:09:46 +0200
The field taking the value 7 (resulting in 18-bit DIDs when using the
calculation in cap_ndoms(), when the DID fields are only 16 bits wide)
is reserved. Instead of misbehaving in case we would encounter such an
IOMMU, refuse to use it.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
master commit: a1545fbf45
master date: 2022-04-20 10:54:26 +0200
A hypervisor built without CONFIG_GDBSX will crash in case the
XEN_DOMCTL_gdbsx_guestmemio domctl is being called, as the call will
end up in iommu_do_domctl() with d == NULL:
(XEN) CPU: 6
(XEN) RIP: e008:[<ffff82d040269984>] iommu_do_domctl+0x4/0x30
(XEN) RFLAGS: 0000000000010202 CONTEXT: hypervisor (d0v0)
(XEN) rax: 00000000000003e8 rbx: ffff830856277ef8 rcx: ffff830856277fff
...
(XEN) Xen call trace:
(XEN) [<ffff82d040269984>] R iommu_do_domctl+0x4/0x30
(XEN) [<ffff82d04035cd5f>] S arch_do_domctl+0x7f/0x2330
(XEN) [<ffff82d040239e46>] S do_domctl+0xe56/0x1930
(XEN) [<ffff82d040238ff0>] S do_domctl+0/0x1930
(XEN) [<ffff82d0402f8c59>] S pv_hypercall+0x99/0x110
(XEN) [<ffff82d0402f5161>] S arch/x86/pv/domain.c#_toggle_guest_pt+0x11/0x90
(XEN) [<ffff82d040366288>] S lstar_enter+0x128/0x130
(XEN)
(XEN) Pagetable walk from 0000000000000144:
(XEN) L4[0x000] = 0000000000000000 ffffffffffffffff
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 6:
(XEN) FATAL PAGE FAULT
(XEN) [error_code=0000]
(XEN) Faulting linear address: 0000000000000144
(XEN) ****************************************
It used to be permitted to pass DOMID_IDLE to dbg_rw_mem(), which is why the
special case skipping the domid checks exists. Now that it is only permitted
to pass proper domids, remove the special case, making 'd' always valid.
Reported-by: Cheyenne Wills <cheyenne.wills@gmail.com>
Fixes: e726a82ca0 ("xen: make gdbsx support configurable")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: f00daf1fb3
master date: 2022-04-19 17:07:08 +0100
xsm_unmap_domain_irq was seen denying unmap_domain_pirq when called from
complete_domain_destroy as an RCU callback. The source context was an
unexpected, random domain. Since this is a xen-internal operation,
going through the XSM hook is inapproriate.
Check d->is_dying and skip the XSM hook when set since this is a cleanup
operation for a domain being destroyed.
Suggested-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: 2e6f95a942
master date: 2022-04-08 14:51:52 +0200
Track whether symbols belong to ignored sections in order to avoid
applying relocations referencing those symbols. The address of such
symbols won't be resolved and thus the relocation will likely fail or
write garbage to the destination.
Return an error in that case, as leaving unresolved relocations would
lead to malfunctioning payload code.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Tested-by: Bjoern Doebel <doebel@amazon.de>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
master commit: 9120b5737f
master date: 2022-04-08 10:27:11 +0200
A side effect of ignoring such sections is that symbols belonging to
them won't be resolved, and that could make relocations belonging to
other sections that reference those symbols fail.
For example it's likely to have an empty .altinstr_replacement with
symbols pointing to it, and marking the section as ignored will
prevent the symbols from being resolved, which in turn will cause any
relocations against them to fail.
In order to solve this do not ignore sections with 0 size, only ignore
sections that don't have the SHF_ALLOC flag set.
Special case such empty sections in move_payload so they are not taken
into account in order to decide whether a livepatch can be safely
re-applied after a revert.
Fixes: 98b728a7b2 ('livepatch: Disallow applying after an revert')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Tested-by: Bjoern Doebel <doebel@amazon.de>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
master commit: 0dc1f929e8
master date: 2022-04-08 10:24:10 +0200
%pp wants the address of an SBDF, not that of a PCI device.
Fixes: b4f2116060 ("vpci/msix: fix PBA accesses")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: d3f61beea4
master date: 2022-04-07 18:01:24 +0200
c/s 1a914256dc increased the AMD max leaf from 0x8000001c to 0x80000021, but
did not adjust anything in the calculate_*_policy() chain. As a result, on
hardware supporting these leaves, we read the real hardware values into the
raw policy, then copy into host, and all the way into the PV/HVM default
policies.
All 4 of these leaves have enable bits (first two by TopoExt, next by SEV,
next by PQOS), so any software following the rules is fine and will leave them
alone. However, leaf 0x8000001d takes a subleaf input and at least two
userspace utilities have been observed to loop indefinitely under Xen (clearly
waiting for eax to report "no more cache levels").
Such userspace is buggy, but Xen's behaviour isn't great either.
In the short term, clobber all information in these leaves. This is a giant
bodge, but there are complexities with implementing all of these leaves
properly.
Fixes: 1a914256dc ("x86/cpuid: support LFENCE always serialising CPUID bit")
Link: https://github.com/QubesOS/qubes-issues/issues/7392
Reported-by: fosslinux <fosslinux@aussies.space>
Reported-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: d4012d5008
master date: 2022-04-07 11:36:45 +0100
Andrew Cooper
Jan Beulich
Anthony PERARD
Charles Arnold
Julien Grall
Marek Marczykowski-Górecki
Roger Pau Monné
Juergen Gross
David Vrabel
Jason Andryuk