Right now, they're the same underlying type, containing disjoint information.
Drop the d->arch.msr pointer, and union d->arch.cpuid to give it a second name
of cpu_policy in the interim.
Merge init_domain_{cpuid,msr}_policy() into a single init_domain_cpu_policy(),
moving the implementation into cpu-policy.c
No practical change. This undoes the transient doubling of storage space from
earlier patches.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit bd13dae348)
Right now, they're the same underlying type, containing disjoint information.
Introduce a new cpu-policy.{h,c} to be the new location for all policy
handling logic. Place the combined objects in __ro_after_init, which is new
since the original logic was written.
As we're trying to phase out the use of struct old_cpu_policy entirely, rework
update_domain_cpu_policy() to not pointer-chase through system_policies[].
This in turn allows system_policies[] in sysctl.c to become static and reduced
in scope to XEN_SYSCTL_get_cpu_policy.
No practical change. This undoes the transient doubling of storage space from
earlier patches.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 6bc3336679)
As with the cpuid side, use a temporary define to make struct msr_policy still
work.
Note, this means that domains now have two separate struct cpu_policy
allocations with disjoint information, and system policies are in a similar
position, as well as xc_cpu_policy objects in libxenguest. All of these
duplications will be addressed in the following patches.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 03812da375)
Also merge lib/x86/cpuid.h entirely into lib/x86/cpu-policy.h
Use a temporary define to make struct cpuid_policy still work.
There's one forward declaration of struct cpuid_policy in
tools/tests/x86_emulator/x86-emulate.h that isn't covered by the define, and
it's easier to rename that now than to rearrange the includes.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 743e530380)
These weren't great names to begin with, and using {leaves,msrs} matches up
better with the existing nr_{leaves,msr} parameters anyway.
Furthermore, by renaming these fields we can get away with using some #define
trickery to avoid the struct {cpuid,msr}_policy merge needing to happen in a
single changeset.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 21e3ef57e0)
xen: Correct comments after renaming xen_{dom,sys}ctl_cpu_policy fields
Fixes: 21e3ef57e0 ("x86: Rename {domctl,sysctl}.cpu_policy.{cpuid,msr}_policy fields")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
(cherry picked from commit 6e06d229d5)
We want to merge struct cpuid_policy and struct msr_policy together, and the
result wants to be called struct cpu_policy.
The current struct cpu_policy, being a pair of pointers, isn't terribly
useful. Rename the type to struct old_cpu_policy, but it will disappear
entirely once the merge is complete.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit c2ec94c370)
Featuresets are supposed to be disappearing when the CPU policy infrastructure
is complete, but that has taken longer than expected, and isn't going to be
complete imminently either.
In the meantime, Xen does have proper default/max featuresets, and xen-cpuid
can even get them via the XEN_SYSCTL_cpu_policy_* interface, but only knows
now to render them nicely via the featureset interface.
Differences between default and max are a frequent source of errors,
frequently too in secret leading up to an embargo, so extend the featureset
sysctl to allow xen-cpuid to render them all nicely.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
(cherry picked from commit 433d012c6c)
struct fsinfo is the vestigial remnant of an older internal design which
didn't survive very long.
Simplify things by inlining get_featureset() and having a single memory
allocation that gets reused. This in turn changes featuresets[] to be a
simple list of names, so rename it to fs_names[].
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit ec3474e1dd)
Gitlab reports:
init.c:348:18: error: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Werror,-Wsingle-bit-bitfield-constant-conversion]
ctrl->is_server = 1;
^ ~
1 error generated.
make[6]: *** [/builds/xen-project/people/andyhhp/xen/tools/libs/vchan/../../../tools/Rules.mk:188: init.o] Error 1
In Xen 4.18, this was fixed with c/s 99ab02f63e ("tools: convert bitfields
to unsigned type") but this is an ABI change which can't be backported.
Swich 1 for -1 to provide a minimally invasive way to fix the build.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
The opensuse-tumbleweed build jobs currently fail with:
/builds/xen-project/xen/stubdom/tpm_emulator-x86_64/crypto/rsa.c: In function 'rsa_private':
/builds/xen-project/xen/stubdom/tpm_emulator-x86_64/crypto/rsa.c:56:7: error: the comparison will always evaluate as 'true' for the address of 'p' will never be NULL [-Werror=address]
56 | if (!key->p || !key->q || !key->u) {
| ^
In file included from /builds/xen-project/xen/stubdom/tpm_emulator-x86_64/crypto/rsa.c:17:
/builds/xen-project/xen/stubdom/tpm_emulator-x86_64/crypto/rsa.h:28:12: note: 'p' declared here
28 | tpm_bn_t p;
| ^
This is because all tpm_bn_t's are 1-element arrays (of either a GMP or
OpenSSL BIGNUM flavour).
Adjust it to compile. No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
(cherry picked from commit 46c5ef609b)
According to openpty(3) it is required to include <pty.h> to get the
prototypes for openpty() and login_tty(). But this is not what the
function AX_CHECK_PTYFUNCS actually does. It makes no attempt to include
the required header.
The two source files which call openpty() and login_tty() already contain
the conditionals to include the required header.
Remove the bogus m4 file to fix build with clang, which complains about
calls to undeclared functions.
Remove usage of INCLUDE_LIBUTIL_H in libxl_bootloader.c, it is already
covered by inclusion of libxl_osdep.h.
Remove usage of PTYFUNCS_LIBS in libxl/Makefile, it is already covered
by UTIL_LIBS from config/StdGNU.mk.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
(cherry picked from commit 42abf5b9c5)
As specified on Errata 1474:
"A core will fail to exit CC6 after about 1044 days after the last
system reset. The time of failure may vary depending on the spread
spectrum and REFCLK frequency."
Detect when running on AMD Zen2 and setup a timer to prevent entering
C6 after 1000 days of uptime. Take into account the TSC value at boot
in order to account for any time elapsed before Xen has been booted.
Worst case we end up disabling C6 before strictly necessary, but that
would still be safe, and it's better than not taking the TSC value
into account and hanging.
Disable C6 by updating the MSR listed in the revision guide, this
avoids applying workarounds in the CPU idle drivers, as the processor
won't be allowed to enter C6 by the hardware itself.
Print a message once C6 is disabled in order to let the user know.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: f7065b24f4
master date: 2023-07-31 15:05:48 +0200
The fix for XSA-417 had a bug: domain_alloc_permrefs() will not return
a negative value in case of an error, but a plain errno value.
Note this is not considered to be a security issue, as the only case
where domain_alloc_permrefs() will return an error is a failed memory
allocation. As a guest should not be able to drive Xenstore out of
memory, this is NOT a problem a guest can trigger at will.
Fixes: ab12821822 ("tools/xenstore: fix checking node permissions")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
master commit: 0c53c638e1
master date: 2023-07-21 08:32:43 +0200
Loading the VGA base address involves sym_esi(), i.e. %esi still needs
to hold the relocation base address. Therefore the address of the
message to output cannot be "passed" in %esi. Put the message offset in
%ecx instead, adding it into %esi _after_ its last use as base address.
Fixes: b28044226e ("x86: make Xen early boot code relocatable")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: b1c16800e5
master date: 2023-07-19 10:22:56 +0200
`Tag_cons` is `0` and is meant to be used as the tag argument for
`caml_alloc`/`caml_alloc_small` when constructing a non-empty list.
The empty list is `Val_emptylist` instead, which is really just `Val_int(0)`.
Assigning `0` to a list value like this is equivalent to assigning the naked
pointer `NULL` to the field. Naked pointers are not valid in OCaml 5, however
even in OCaml <5.x any attempt to iterate on the list will lead to a segfault.
The list currently only has an opaque type, so no code would have reason to
iterate on it currently, but we shouldn't construct invalid OCaml values that
might lead to a crash when exploring the type.
`Val_emptylist` is available since OCaml 3.01 as a constant.
Fixes: e5ac68a011 ("x86/hvm: Revert per-domain APIC acceleration support")
Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
master commit: 99e4554893
master date: 2023-07-13 11:06:07 +0100
Cortex-A77 cores (r0p0, r1p0) could deadlock on a sequence of a
store-exclusive or read of PAR_EL1 and a load with device or non-cacheable
memory attributes.
A workaround is available, but it depends on a firmware counterpart.
The proposed workaround from the errata document is to modify the software
running at EL1 and above to include a DMB SY before and after accessing
PAR_EL1.
In conjunction to the above, the firmware needs to use a specific write
sequence to several IMPLEMENTATION DEFINED registers to have the hardware
insert a DMB SY after all load-exclusive and store-exclusive instructions.
Apply the workaround to Xen where PAR_EL1 is read, implementing an helper
function to do that.
Since Xen can be interrupted by irqs in any moment, add a barrier on
entry/exit when we are running on the affected cores.
A guest without the workaround can deadlock the system, so warn the users
of Xen with the above type of cores to use only trusted guests, by
printing a message on Xen startup.
This is XSA-436 / CVE-2023-34320.
Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
[stefano: add XSA-436 to commit message]
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
This line:
val &= ~chickenbit;
ends up truncating val to 32 bits, and turning off various errata workarounds
in Zen2 systems.
Fixes: f91c5ea970 ("x86/amd: Mitigations for Zenbleed")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit c0dd53b8cb)
In the follow-up patch we will add new jobs using debug Xen builds.
Because the debug builds take more space and we might end up in
a situation when there is not enough free space (especially during
a static memory test that reserves some region in the middle), increase
RAM size for QEMU from 1GB to 2GB.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
(cherry picked from commit a0030a83e8)
Zenbleed is a malfunction on AMD Zen2 uarch parts which results in corruption
of the vector registers. An attacker can trigger this bug deliberately in
order to access stale data in the physical vector register file. This can
include data from sibling threads, or a higher-privilege context.
Microcode is the preferred mitigation but in the case that's not available use
the chickenbit as instructed by AMD. Re-evaluate the mitigation on late
microcode load too.
This is XSA-433 / CVE-2023-20593.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
(cherry picked from commit f91c5ea970)
Clang-15 (as seen in the FreeBSD 14 tests) complains:
xg_main.c:1248 error: a function declaration without a
prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
xg_init()
^
void
The error message is a bit confusing but appears to new as part of
-Wdeprecated-non-prototype which is part of supporting C2x which formally
removes K&R syntax.
Either way, fix the identified function.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
master commit: e2312e41f0
master date: 2023-02-17 11:01:54 +0000
Clang-15 (as seen in the FreeBSD 14 tests) complains:
arch/x86/time.c:1364:20: error: a function declaration without a
prototype is deprecated in all versions of C [-Werror,-Wstrict-prototypes]
s_time_t get_s_time()
^
void
The error message is a bit confusing but appears to new as part of
-Wdeprecated-non-prototype which is part of supporting C2x which formally
removes K&R syntax.
Either way, fix the identified functions.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 22b2fa4766
master date: 2023-02-17 11:01:54 +0000
Fix two issues related to leaf address lookups in VT-d:
* When translating an address that falls inside of a superpage in the
IOMMU page tables the fetching of the PTE value wasn't masking of the
contiguous related data, which caused the returned data to be
corrupt as it would contain bits that the caller would interpret as
part of the address.
* When the requested leaf address wasn't mapped by a superpage the
returned value wouldn't have any of the low 12 bits set, thus missing
the permission bits expected by the caller.
Take the opportunity to also adjust the function comment to note that
when returning the full PTE the bits above PADDR_BITS are removed.
Fixes: c71e55501a ('VT-d: have callers specify the target level for page table walks')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
master commit: 82b28deb25
master date: 2023-06-19 15:46:03 +0200
The iommu local variable does not point to to a valid amd_iommu element
after the call to for_each_amd_iommu(). Instead check whether any IOMMU
on the system doesn't support Invalidate All in order to perform the
per-domain and per-device flushes.
Fixes: 9c46139de8 ('amd iommu: Support INVALIDATE_IOMMU_ALL command.')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 5ecbb77974
master date: 2023-06-13 14:41:32 +0200
microcode_update_helper() may return early while holding
cpu_add_remove_lock, hence preventing any writers from taking it again.
Leave through the `put` label instead so it's properly released.
Fixes: 5ed12565aa ("microcode: rendezvous CPUs in NMI handler and load ucode")
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: b35b22acb8
master date: 2023-06-05 16:11:10 +0100
When traversing the list of pci devices assigned to a domain cope with
some of them not having the vpci struct allocated. It should be
possible for the hardware domain to have read-only devices assigned
that are not handled by vPCI, such support will be added by further
patches.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: ee045f3a4a
master date: 2023-05-26 09:18:37 +0200
clang complains about the signed type:
implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Wsingle-bit-bitfield-constant-conversion]
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Backport: Dropped the libxenvchan change, for the original commit saying
"The potential ABI change in libxenvchan is covered by the Xen version
based SONAME."
which won't hold on stable trees.
master commit: 99ab02f63e
master date: 2023-05-16 20:03:02 +0100
When a domain parameter is provided to pci_get_pdev() the search
function would match against the bdf, without taking the segment into
account.
Fix this and also account for the passed segment.
Fixes: 8cf6e07389 ('PCI: simplify (and thus correct) pci_get_pdev{,_by_domain}()')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: c7908869ac
master date: 2023-05-22 16:11:55 +0200
When creating a domU, but the creation fails, there is a corner case that may
lead to a crash in the null scheduler when running a debug build of Xen.
(XEN) ****************************************
(XEN) Panic on CPU 0:
(XEN) Assertion 'npc->unit == unit' failed at common/sched/null.c:379
(XEN) ****************************************
The events leading to the crash are:
* null_unit_insert() was invoked with the unit offline. Since the unit was
offline, unit_assign() was not called, and null_unit_insert() returned.
* Later during domain creation, the unit was onlined
* Eventually, domain creation failed due to bad configuration
* null_unit_remove() was invoked with the unit still online. Since the unit was
online, it called unit_deassign() and triggered an ASSERT.
To fix this, only call unit_deassign() when npc->unit is non-NULL in
null_unit_remove.
Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Dario Faggioli <dfaggioli@suse.com>
master commit: c2eae2614c
master date: 2023-05-22 16:11:40 +0200
Or else when iommu_intremap is set to iommu_intremap_full the assert
triggers.
Fixes: 1ba66a870e ('AMD/IOMMU: without XT, x2APIC needs to be forced into physical mode')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 4c507d8a6b
master date: 2023-05-12 09:35:36 +0200
pci_serial_early_init() enables PCI_COMMAND_IO for IO-based UART
devices, add setting PCI_COMMAND_MEMORY for MMIO-based UART devices too.
Note the MMIO-based devices in practice need a "pci" sub-option,
otherwise a few parameters are not initialized (including bar_idx,
reg_shift, reg_width etc). The "pci" is not supposed to be used with
explicit BDF, so do not key setting PCI_COMMAND_MEMORY on explicit BDF
being set. Contrary to the IO-based UART, pci_serial_early_init() will
not attempt to set BAR0 address, even if user provided io_base manually
- in most cases, those are with an offest and the current cmdline syntax
doesn't allow expressing it. Due to this, enable PCI_COMMAND_MEMORY only
if uart->bar is already populated. In similar spirit, this patch does
not support setting BAR0 of the bridge.
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
master commit: a16fb78515
master date: 2023-05-08 14:15:38 +0200
gcc13 fails to track the allocated memory in backup_ptes:
xg_offline_page.c: In function 'backup_ptes':
xg_offline_page.c:191:13: error: pointer 'orig' may be used after 'realloc' [-Werror=use-after-free]
191 | free(orig);
Assist the analyzer by slightly rearranging the code:
In case realloc succeeds, the previous allocation is either extended
or released internally. In case realloc fails, the previous allocation
is left unchanged. Return an error in this case, the caller will
release the currently allocated memory in its error path.
http://bugzilla.suse.com/show_bug.cgi?id=1210570
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Reviewed-by: Juergen Gross <jgross@suse.com>
Compile-tested-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
master commit: 99a9c3d714
master date: 2023-05-03 15:06:41 +0200
While I was the one to introduce it, I don't think it is correct: A
bogus continuation call issued by a tool stack domain may find another
continuation in progress. IOW we've been asserting caller controlled
state (which is reachable only via a domctl), and the early (lock-less)
check in paging_domctl() helps in a limited way only.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: 0956aa2219
master date: 2023-05-03 13:38:30 +0200
In case XSM is active, the handling of XEN_SYSCTL_getdomaininfolist
can fail if the last domain scanned isn't allowed to be accessed by
the calling domain (i.e. xsm_getdomaininfo(XSM_HOOK, d) is failing).
Fix that by just ignoring scanned domains where xsm_getdomaininfo()
is returning an error, like it is effectively done when such a
situation occurs for a domain not being the last one scanned.
Fixes: d046f361dc ("Xen Security Modules: XSM")
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: b033eddc97
master date: 2023-05-02 12:04:58 +0200
Some firmware/devices are found to not reset MSI-X properly, leaving
MASKALL set. Jason reports on his machine MASKALL persists through a
warm reboot, but is cleared on cold boot. Xen relies on initial state
being MASKALL clear. Especially, pci_reset_msix_state() assumes if
MASKALL is set, it was Xen setting it due to msix->host_maskall or
msix->guest_maskall. Clearing just MASKALL is risky if ENABLE is set,
so clear them both.
Reported-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Jason Andryuk <jandryuk@gmail.com>
master commit: 913751d7af
master date: 2023-04-25 12:16:17 +0200
In COVERAGE=y but DEBUG=n builds (observed by randconfig testing) gcc12
takes issue with the subtraction of 1 from __stop___pre_ex_table[],
considering this an out of bounds access. Not being able to know that
the symbol actually marks the end of an array, the compiler is kind of
right with this diagnosis. Move the subtraction into the function.
Reported-by: Anthony Perard <anthony.perard@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 353b8cc568
master date: 2023-02-22 13:50:20 +0100
The current logic to set SSBD on AMD Family 17h and Hygon Family 18h
processors requires that the setting of SSBD is coordinated at a core
level, as the setting is shared between threads. Logic was introduced
to keep track of how many threads require SSBD active in order to
coordinate it, such logic relies on using a per-core counter of
threads that have SSBD active.
Given the current logic, it's possible for a guest to under or
overflow the thread counter, because each write to VIRT_SPEC_CTRL.SSBD
by the guest gets propagated to the helper that does the per-core
active accounting. Overflowing the counter is not so much of an
issue, as this would just make SSBD sticky.
Underflowing however is more problematic: on non-debug Xen builds a
guest can perform empty writes to VIRT_SPEC_CTRL that would cause the
counter to underflow and thus the value gets saturated to the max
value of unsigned int. At which points attempts from any thread to
set VIRT_SPEC_CTRL.SSBD won't get propagated to the hardware anymore,
because the logic will see that the counter is greater than 1 and
assume that SSBD is already active, effectively loosing the setting
of SSBD and the protection it provides.
Fix this by introducing a per-CPU variable that keeps track of whether
the current thread has legacy SSBD active or not, and thus only
attempt to propagate the value to the hardware once the thread
selected value changes.
This is XSA-431 / CVE-2022-42336
Fixes: b2030e6730 ('amd/virt_ssbd: set SSBD at vCPU context switch')
Reported-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: eda98ea870
master date: 2023-05-16 17:17:24 +0200
As a result of 241702e064 the
mfn_valid() check in sh_unshadow_for_p2m_change() was lost. That
allows sh_remove_shadows() to be called with gfns that have no backing
page, causing an ASSERT to trigger in debug builds or dereferencing an
arbitrary pointer partially under guest control in non-debug builds:
RIP: e008:[<ffff82d0402dcf2c>] sh_remove_shadows+0x19f/0x722
RFLAGS: 0000000000010246 CONTEXT: hypervisor (d0v2)
[...]
Xen call trace:
[<ffff82d0402dcf2c>] R sh_remove_shadows+0x19f/0x722
[<ffff82d0402e28f4>] F arch/x86/mm/shadow/hvm.c#sh_unshadow_for_p2m_change+0xab/0x2b7
[<ffff82d040311931>] F arch/x86/mm/p2m-pt.c#write_p2m_entry+0x19b/0x4d3
[<ffff82d0403131b2>] F arch/x86/mm/p2m-pt.c#p2m_pt_set_entry+0x67b/0xa8e
[<ffff82d040302c92>] F p2m_set_entry+0xcc/0x149
[<ffff82d040305a50>] F unmap_mmio_regions+0x17b/0x2c9
[<ffff82d040241e5e>] F do_domctl+0x11f3/0x195e
[<ffff82d0402c7e10>] F hvm_hypercall+0x5b1/0xa2d
[<ffff82d0402adc72>] F vmx_vmexit_handler+0x130f/0x1cd5
[<ffff82d040203602>] F vmx_asm_vmexit_handler+0xf2/0x210
****************************************
Panic on CPU 1:
Assertion 'mfn_valid(gmfn)' failed at arch/x86/mm/shadow/common.c:2203
****************************************
Fix this by restoring the mfn_valid() check in
sh_unshadow_for_p2m_change(), unifying it with the rest of the checks
that are done at the start of the function.
This is XSA-430 / CVE-2022-42335
Fixes: 241702e064 ('x86/shadow: slightly consolidate sh_unshadow_for_p2m_change() (part II)')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: f6c3cb2162
master date: 2023-04-25 15:44:54 +0200
Now, when these packages are already installed in the respective
containers, we can remove them from the test scripts.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
master commit: 72cfe1c3ad
master date: 2022-12-09 14:55:33 -0800
It is rude to hide width formatting inside a PRI* macro, doubly so when it's
only in one bitness of the macro.
However its fully buggy when all the users use %#"PRI because then it expands
to the common trap of %#08x which does not do what the author intends.
Switch the 32bit ELF PRI formatters to use plain integer PRI's, just like on
the 64bit side already. No practical change.
Fixes: 7597fabca7 ("livepatch: Include sizes when an mismatch occurs")
Fixes: 380b229634 ("xsplice: Implement payload loading")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
master commit: cfa2bb82c0
master date: 2023-04-19 15:55:29 +0100
Right now, trying to apply a livepatch on any system with CET shstk (AMD Zen3
or later, Intel Tiger Lake or Sapphire Rapids and later) fails as follows:
(XEN) livepatch: lp: Verifying enabled expectations for all functions
(XEN) common/livepatch.c:1591: livepatch: lp: timeout is 30000000ns
(XEN) common/livepatch.c:1703: livepatch: lp: CPU28 - IPIing the other 127 CPUs
(XEN) livepatch: lp: Applying 1 functions
(XEN) hi_func: Hi! (called 1 times)
(XEN) Hook executing.
(XEN) Assertion 'local_irq_is_enabled() || cpumask_subset(mask, cpumask_of(cpu))' failed at arch/x86/smp.c:265
(XEN) *** DOUBLE FAULT ***
<many double faults>
The assertion failure is from a global (system wide) TLB flush initiated by
modify_xen_mappings(). I'm not entirely sure when this broke, and I'm not
sure exactly what causes the #DF's, but it doesn't really matter either
because they highlight a latent bug that I'd overlooked with the CET-SS vs
patching work the first place.
While we're careful to arrange for the patching CPU to avoid encountering
non-shstk memory with transient shstk perms, other CPUs can pick these
mappings up too if they need to re-walk for uarch reasons.
Another bug is that for livepatching, we only disable CET if shadow stacks are
in use. Running on Intel CET systems when Xen is only using CET-IBT will
crash in arch_livepatch_quiesce() when trying to clear CR0.WP with CR4.CET
still active.
Also, we never went and cleared the dirty bits on .rodata. This would
matter (for the same reason it matters on .text - it becomes a valid target
for WRSS), but we never actually patch .rodata anyway.
Therefore rework how we do patching for both alternatives and livepatches.
Introduce modify_xen_mappings_lite() with a purpose similar to
modify_xen_mappings(), but stripped down to the bare minimum as it's used in
weird contexts. Leave all complexity to the caller to handle.
Instead of patching by clearing CR0.WP (and having to jump through some
fragile hoops to disable CET in order to do this), just transiently relax the
permissions on .text via l2_identmap[].
Note that neither alternatives nor livepatching edit .rodata, so we don't need
to relax those permissions at this juncture.
The perms are relaxed globally, but this is safe enough. Alternatives run
before we boot APs, and Livepatching runs in a quiesced state where the other
CPUs are not doing anything interesting.
This approach is far more robust.
Fixes: 48cdc15a42 ("x86/alternatives: Clear CR4.CET when clearing CR0.WP")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
master commit: 8676092a0f
master date: 2023-04-18 20:20:26 +0100
The Long Mode consistency checks exist to "ensure that the processor does not
enter an undefined mode or state that results in unpredictable behavior". APM
Vol2 Table 14-5 "Long-Mode Consistency Checks" lists them, but there is no row
preventing the OS from trying to exit Long mode while in 64bit mode. This
could leave the CPU in Protected Mode with an %rip above the 4G boundary.
Experimentally, AMD CPUs really do permit this state transition. An OS which
tries it hits an instant SHUTDOWN, even in cases where the truncation I expect
to be going on behind the scenes ought to result in sane continued execution.
Furthermore, right from the very outset, the APM Vol2 14.7 "Leaving Long Mode"
section instructs peoples to switch to a compatibility mode segment first
before clearing CR0.PG, which does clear out the upper bits in %rip. This is
further backed up by Vol2 Figure 1-6 "Operating Modes of the AMD64
Architecture".
Either way, this appears to have been a genuine oversight in the AMD64 spec.
Intel, on the other hand, rejects this state transition with #GP.
Between revision 71 (Nov 2019) and 72 (May 2020) of SDM Vol3, a footnote to
4.1.2 "Paging-Mode Enable" was altered from
If CR4.PCIDE= 1, an attempt to clear CR0.PG causes a general-protection
exception (#GP); software should clear CR4.PCIDE before attempting to
disable paging.
to
If the logical processor is in 64-bit mode or if CR4.PCIDE= 1, an attempt to
clear CR0.PG causes a general-protection exception (#GP). Software should
transition to compatibility mode and clear CR4.PCIDE before attempting to
disable paging.
which acknowledges this corner case, but there doesn't appear to be any other
discussion even in the relevant Long Mode sections.
So it appears that Intel spotted and addressed the corner case in IA-32e mode,
but were 15 years late to document it.
Xen was written to the AMD spec, and misses the check. Follow the Intel
behaviour, because it is more sensible and avoids hitting a VMEntry failure.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: 18c128ba66
master date: 2023-04-14 18:18:20 +0100
Based on observations on a fair range of hardware from both primary
vendors even zero-iteration-count instances of these insns perform the
port related permission checking first.
Fixes: fe30060046 ("x86: Fix emulation of REP prefix")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: f41c88a6fc
master date: 2023-03-30 13:07:16 +0200
Today when finalizing a transaction the number of node quota is checked
to not being exceeded after the transaction. This check is always done,
even if the transaction is being performed by a privileged connection,
or if there were no nodes created in the transaction.
Correct that by checking quota only if:
- the transaction is being performed by an unprivileged guest, and
- at least one node was created in the transaction
Reported-by: Julien Grall <julien@xen.org>
Fixes: f2bebf72c4 ("xenstore: rework of transaction handling")
Signed-off-by: Juergen Gross <jgross@suse.com>
master commit: f6b801c36b
master date: 2023-03-29 22:02:36 +0100
For similar reasons to c/s a6b1e2b80f. While this container is still
build-able for now, all the other problems with explicitly-versioned compilers
remain.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
(cherry picked from commit 7a29837572)
In the interest of having less jobs, we remove the x86_32 build jobs
that do release build. Debug build is very likely to be enough to find
32bit build issues.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
(cherry picked from commit 7b66792ea7)
Andrew Cooper
Olaf Hering
Khem Raj
Roger Pau Monné
Juergen Gross
Jan Beulich
Edwin Török
Luca Fancellu
Michal Orzel
Alejandro Vallejo
Stewart Hildebrand
Yann Dirson
Marek Marczykowski-Górecki
Anthony PERARD