I can only guess that using block_speculation() there was a leftover
from, earlier on, SPECULATIVE_HARDEN_LOCK depending on
SPECULATIVE_HARDEN_BRANCH.
Fixes: 197ecd838a ("locking: attempt to ensure lock wrappers are always inline")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 62018f0870
master date: 2024-03-18 13:53:37 +0100
Use a snapshot which includes commit
b0ded89e917b48b73097d3b8b88dfa3afb264ed0 ("[build] Disable dangling
pointer checking for GCC"), which fixes build with gcc12.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 18a36b4a9b
master date: 2023-04-25 23:47:45 +0100
Conditionally taken locks that use the pattern:
if ( lock )
spin_lock(...);
Need an else branch in order to issue an speculation barrier in the else case,
just like it's done in case the lock needs to be acquired.
eval_nospec() could be used on the condition itself, but that would result in a
double barrier on the branch where the lock is taken.
Introduce a new pair of helpers, {gfn,spin}_lock_if() that can be used to
conditionally take a lock in a speculation safe way.
This is part of XSA-453 / CVE-2024-2193
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 03cf7ca23e)
Add a speculation barrier to the clearly identified open-coded lock taking
functions.
Note that the memory sharing page_lock() replacement (_page_lock()) is left
as-is, as the code is experimental and not security supported.
This is part of XSA-453 / CVE-2024-2193
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 42a572a38e)
In order to prevent the locking speculation barriers from being inside of
`call`ed functions that could be speculatively bypassed.
While there also add an extra locking barrier to _mm_write_lock() in the branch
taken when the lock is already held.
Note some functions are switched to use the unsafe variants (without speculation
barrier) of the locking primitives, but a speculation barrier is always added
to the exposed public lock wrapping helper. That's the case with
sched_spin_lock_double() or pcidevs_lock() for example.
This is part of XSA-453 / CVE-2024-2193
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 197ecd838a)
Add direct calls to block_lock_speculation() where required in order to prevent
speculation into the lock protected critical regions. Also convert
_percpu_read_lock() from inline to always_inline.
Note that _percpu_write_lock() has been modified the use the non speculation
safe of the locking primites, as a speculation is added unconditionally by the
calling wrapper.
This is part of XSA-453 / CVE-2024-2193
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit f218daf6d3)
Introduce inline wrappers as required and add direct calls to
block_lock_speculation() in order to prevent speculation into the rwlock
protected critical regions.
Note the rwlock primitives are adjusted to use the non speculation safe variants
of the spinlock handlers, as a speculation barrier is added in the rwlock
calling wrappers.
trylock variants are protected by using lock_evaluate_nospec().
This is part of XSA-453 / CVE-2024-2193
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit a1fb15f616)
Introduce a new Kconfig option to block speculation into lock protected
critical regions. The Kconfig option is enabled by default, but the mitigation
won't be engaged unless it's explicitly enabled in the command line using
`spec-ctrl=lock-harden`.
Convert the spinlock acquire macros into always-inline functions, and introduce
a speculation barrier after the lock has been taken. Note the speculation
barrier is not placed inside the implementation of the spin lock functions, as
to prevent speculation from falling through the call to the lock functions
resulting in the barrier also being skipped.
trylock variants are protected using a construct akin to the existing
evaluate_nospec().
This patch only implements the speculation barrier for x86.
Note spin locks are the only locking primitive taken care in this change,
further locking primitives will be adjusted by separate changes.
This is part of XSA-453 / CVE-2024-2193
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 7ef0084418)
Wherever possible, it is a good idea to NULL out the visible reference to an
object prior to freeing it. The FREE*() macros already collect together both
parts, making it easy to adjust.
This has a marginal code generation improvement, as some of the calls to the
free() function can be tailcall optimised.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit c4f427ec87)
Nicola reports that the XSA-438 fix introduced new MISRA violations because of
some incidental tidying it tried to do. The parameter is useless, so resolve
the MISRA regression by removing it.
hap_update_cr3() discards the parameter entirely, while sh_update_cr3() uses
it to distinguish internal and external callers and therefore whether the
paging lock should be taken.
However, we have paging_lock_recursive() for this purpose, which also avoids
the ability for the shadow internal callers to accidentally not hold the lock.
Fixes: fb0ff49fe9 ("x86/shadow: defer releasing of PV's top-level shadow reference")
Reported-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
(cherry picked from commit e71157d1ac)
RFDS affects Atom cores, also branded E-cores, between the Goldmont and
Gracemont microarchitectures. This includes Alder Lake and Raptor Lake hybrid
clien systems which have a mix of Gracemont and other types of cores.
Two new bits have been defined; RFDS_CLEAR to indicate VERW has more side
effets, and RFDS_NO to incidate that the system is unaffected. Plenty of
unaffected CPUs won't be getting RFDS_NO retrofitted in microcode, so we
synthesise it. Alder Lake and Raptor Lake Xeon-E's are unaffected due to
their platform configuration, and we must use the Hybrid CPUID bit to
distinguish them from their non-Xeon counterparts.
Like MD_CLEAR and FB_CLEAR, RFDS_CLEAR needs OR-ing across a resource pool, so
set it in the max policies and reflect the host setting in default.
This is part of XSA-452 / CVE-2023-28746.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit fb5b6f6744)
... before we add yet more complexity to this logic. Mostly expanded
comments, but with three minor changes.
1) Introduce cpu_has_useful_md_clear to simplify later logic in this patch and
future ones.
2) We only ever need SC_VERW_IDLE when SMT is active. If SMT isn't active,
then there's no re-partition of pipeline resources based on thread-idleness
to worry about.
3) The logic to adjust HVM VERW based on L1D_FLUSH is unmaintainable and, as
it turns out, wrong. SKIP_L1DFL is just a hint bit, whereas opt_l1d_flush
is the relevant decision of whether to use L1D_FLUSH based on
susceptibility and user preference.
Rewrite the logic so it can be followed, and incorporate the fact that when
FB_CLEAR is visible, L1D_FLUSH isn't a safe substitution.
This is part of XSA-452 / CVE-2023-28746.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 1eb91a8a06)
VERW is going to be used for a 3rd purpose, and the existing nomenclature
didn't survive the Stale MMIO issues terribly well.
Rename the command line option from `md-clear=` to `verw=`. This is more
consistent with other options which tend to be named based on what they're
doing, not which feature enumeration they use behind the scenes. Retain
`md-clear=` as a deprecated alias.
Rename opt_md_clear_{pv,hvm} and opt_fb_clear_mmio to opt_verw_{pv,hvm,mmio},
which has a side effect of making spec_ctrl_init_domain() rather clearer to
follow.
No functional change.
This is part of XSA-452 / CVE-2023-28746.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit f7603ca252)
On parts vulnerable to RFDS, VERW's side effects are extended to scrub all
non-architectural entries in various Physical Register Files. To remove all
of Xen's values, the VERW must be after popping the GPRs.
Rework SPEC_CTRL_COND_VERW to default to an CPUINFO_error_code %rsp position,
but with overrides for other contexts. Identify that it clobbers eflags; this
is particularly relevant for the SYSRET path.
For the IST exit return to Xen, have the main SPEC_CTRL_EXIT_TO_XEN put a
shadow copy of spec_ctrl_flags, as GPRs can't be used at the point we want to
issue the VERW.
This is part of XSA-452 / CVE-2023-28746.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 0a666cf2cd)
Broken out of the following patch because this change is subtle enough on its
own. See it for the rational of why we're moving VERW.
As for how, extend the trick already used to hold one condition in
flags (RESUME vs LAUNCH) through the POPing of GPRs.
Move the MOV CR earlier. Intel specify flags to be undefined across it.
Encode the two conditions we want using SF and PF. See the code comment for
exactly how.
Leave a comment to explain the lack of any content around
SPEC_CTRL_EXIT_TO_VMX, but leave the block in place. Sods law says if we
delete it, we'll need to reintroduce it.
This is part of XSA-452 / CVE-2023-28746.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 475fa20b73)
restore_all_guest() does a lot of manipulation of the stack after popping the
GPRs, and uses raw %rsp displacements to do so. Also, almost all entrypaths
use raw %rsp displacements prior to pushing GPRs.
Provide better mnemonics, to aid readability and reduce the chance of errors
when editing.
No functional change. The resulting binary is identical.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit 37541208f1)
The current logic to detect when to switch to the next L1 table is incorrectly
using l2_table_offset() in order to notice when the last entry on the current
L1 table has been reached.
It should instead use l1_table_offset() to check whether the index has wrapped
to point to the first entry, and so the next L1 table should be used.
Fixes: 8676092a0f ('x86/livepatch: Fix livepatch application when CET is active')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 7c81558208
master date: 2024-03-11 11:09:42 +0000
BARs of size 2Gb and up can't possibly fit below 4Gb: Both the bottom of
the lower 2Gb range and the top of the higher 2Gb range have special
purpose. Don't even have them influence whether to (perhaps) relocate
low RAM.
Reported-by: Neowutran <xen@neowutran.ovh>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: 57acad12a0
master date: 2024-03-06 10:19:29 +0100
MD_CLEAR and FB_CLEAR need OR-ing across a migrate pool. Allow this, by
having them unconditinally set in max, with the host values reflected in
default. Annotate the bits as having special properies.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: de17162caf
master date: 2024-03-01 20:14:19 +0000
The usage in ALT_CALL_ARG() on clang of:
register union {
typeof(arg) e;
const unsigned long r;
} ...
When `arg` is the first argument to alternative_{,v}call() and
const_vlapic_vcpu() is used results in clang 3.5.0 complaining with:
arch/x86/hvm/vlapic.c:141:47: error: non-const static data member must be initialized out of line
alternative_call(hvm_funcs.test_pir, const_vlapic_vcpu(vlapic), vec) )
Workaround this by pulling `arg1` into a local variable, like it's done for
further arguments (arg2, arg3...)
Originally arg1 wasn't pulled into a variable because for the a1_ register
local variable the possible clobbering as a result of operators on other
variables don't matter:
https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html#Local-Register-Variables
Note clang version 3.8.1 seems to already be fixed and don't require the
workaround, but since it's harmless do it uniformly everywhere.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Fixes: 2ce562b2a4 ('x86/altcall: use a union as register type for function parameters on clang')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
master commit: c20850540a
master date: 2024-02-29 08:21:49 +0100
libxl__spawn_qdisk_backend() explicitly sets guest_config to NULL when
starting QEMU (the usual launch through libxl__spawn_local_dm() has a
guest_config though).
Bail early on a NULL guest_config/d_config. This skips the QMP queries
for chardevs and VNC, but this xenpv QEMU instance isn't expected to
provide those - only qdisk (or 9pfs backends after an upcoming change).
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
master commit: d4f3d35f04
master date: 2024-02-29 08:18:38 +0100
It seems the build variables for those tests where copy-pasted from
xen_action_hooks_marker-objs and not adjusted to use the correct source files.
Fixes: 6047104c3c ('livepatch: Add per-function applied/reverted state tracking marker')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
master commit: e579677095
master date: 2024-02-28 16:57:25 +0000
The purpose of the norevert test is to install a dummy handler that replaces
the internal Xen revert code, and then perform the revert in the post-revert
hook. For that purpose the usage of the previous common_livepatch_revert() is
not enough, as that just reverts specific functions, but not the whole state of
the payload.
Remove both common_livepatch_{apply,revert}() and instead expose
revert_payload{,_tail}() in order to perform the patch revert from the
post-revert hook.
Fixes: 6047104c3c ('livepatch: Add per-function applied/reverted state tracking marker')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
master commit: cdae267ce1
master date: 2024-02-28 16:57:25 +0000
When checking if an address belongs to a patch, or when resolving a symbol,
take into account all loaded livepatch payloads, even if not applied.
This is required in order for the pre-apply and post-revert hooks to work
properly, or else Xen won't detect the instruction pointer belonging to those
hooks as being part of the currently active text.
Move the RCU handling to be used for payload_list instead of applied_list, as
now the calls from trap code will iterate over the payload_list.
Fixes: 8313c864fa ('livepatch: Implement pre-|post- apply|revert hooks')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
master commit: d2daa40fb3
master date: 2024-02-28 16:57:25 +0000
Currently livepatch regions are registered as virtual regions only after the
livepatch has been applied.
This can lead to issues when using the pre-apply or post-revert hooks, as at
that point the livepatch is not in the virtual regions list. If a livepatch
pre-apply hook contains a WARN() it would trigger an hypervisor crash, as the
code to handle the bug frame won't be able to find the instruction pointer that
triggered the #UD in any of the registered virtual regions, and hence crash.
Fix this by adding the livepatch payloads as virtual regions as soon as loaded,
and only remove them once the payload is unloaded. This requires some changes
to the virtual regions code, as the removal of the virtual regions is no longer
done in stop machine context, and hence an RCU barrier is added in order to
make sure there are no users of the virtual region after it's been removed from
the list.
Fixes: 8313c864fa ('livepatch: Implement pre-|post- apply|revert hooks')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
master commit: a57b4074ab
master date: 2024-02-28 16:57:25 +0000
Since the thunk built-in enable is printed as part of the "Compiled-in
support:" line, avoid printing anything in "Xen settings:" if the thunk is
disabled at build time.
Note the BTI-Thunk option printing is also adjusted to print a colon in the
same way the other options on the line do.
Requested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 576528a2a7
master date: 2024-02-27 14:58:40 +0100
Attempt to provide a more helpful error message when the user attempts to set
spec-ctrl=bti-thunk option but the support is build-time disabled.
While there also adjust the command line documentation to mention
CONFIG_INDIRECT_THUNK instead of INDIRECT_THUNK.
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: 8441fa806a
master date: 2024-02-27 14:58:20 +0100
Just like it's done for INDIRECT_THUNK and SHADOW_PAGING.
Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 6e9507f7d5
master date: 2024-02-27 14:57:52 +0100
Tamas reported this UBSAN failure from fuzzing:
(XEN) ================================================================================
(XEN) UBSAN: Undefined behaviour in common/sched/compat.c:48:37
(XEN) left shift of negative value -2147425536
(XEN) ----[ Xen-4.19-unstable x86_64 debug=y ubsan=y Not tainted ]----
...
(XEN) Xen call trace:
(XEN) [<ffff82d040307c1c>] R ubsan.c#ubsan_epilogue+0xa/0xd9
(XEN) [<ffff82d040308afb>] F __ubsan_handle_shift_out_of_bounds+0x11a/0x1c5
(XEN) [<ffff82d040307758>] F compat_set_timer_op+0x41/0x43
(XEN) [<ffff82d04040e4cc>] F hvm_do_multicall_call+0x77f/0xa75
(XEN) [<ffff82d040519462>] F arch_do_multicall_call+0xec/0xf1
(XEN) [<ffff82d040261567>] F do_multicall+0x1dc/0xde3
(XEN) [<ffff82d04040d2b3>] F hvm_hypercall+0xa00/0x149a
(XEN) [<ffff82d0403cd072>] F vmx_vmexit_handler+0x1596/0x279c
(XEN) [<ffff82d0403d909b>] F vmx_asm_vmexit_handler+0xdb/0x200
Left-shifting any negative value is strictly undefined behaviour in C, and
the two parameters here come straight from the guest.
The fuzzer happened to choose lo 0xf, hi 0x8000e300.
Switch everything to be unsigned values, making the shift well defined.
As GCC documents:
As an extension to the C language, GCC does not use the latitude given in
C99 and C11 only to treat certain aspects of signed '<<' as undefined.
However, -fsanitize=shift (and -fsanitize=undefined) will diagnose such
cases.
this was deemed not to need an XSA.
Note: The unsigned -> signed conversion for do_set_timer_op()'s s_time_t
parameter is also well defined. C makes it implementation defined, and GCC
defines it as reduction modulo 2^N to be within range of the new type.
Fixes: 2942f45e09 ("Enable compatibility mode operation for HYPERVISOR_sched_op and HYPERVISOR_set_timer_op.")
Reported-by: Tamas K Lengyel <tamas@tklengyel.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: ae6d4fd876
master date: 2024-02-01 19:52:44 +0000
When running Xen on arm32, in scenario where Xen is loaded at an address
such as boot_phys_offset >= 2GB, UBSAN reports the following:
(XEN) UBSAN: Undefined behaviour in arch/arm/setup.c:739:58
(XEN) pointer operation underflowed 00200000 to 86800000
(XEN) Xen WARN at common/ubsan/ubsan.c:172
(XEN) ----[ Xen-4.19-unstable arm32 debug=y ubsan=y Not tainted ]----
...
(XEN) Xen call trace:
(XEN) [<0031b4c0>] ubsan.c#ubsan_epilogue+0x18/0xf0 (PC)
(XEN) [<0031d134>] __ubsan_handle_pointer_overflow+0xb8/0xd4 (LR)
(XEN) [<0031d134>] __ubsan_handle_pointer_overflow+0xb8/0xd4
(XEN) [<004d15a8>] start_xen+0xe0/0xbe0
(XEN) [<0020007c>] head.o#primary_switched+0x4/0x30
The failure is reported for the following line:
(paddr_t)(uintptr_t)(_start + boot_phys_offset)
This occurs because the compiler treats (ptr + size) with size bigger than
PTRDIFF_MAX as undefined behavior. To address this, switch to macro
virt_to_maddr(), given the future plans to eliminate boot_phys_offset.
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
Tested-by: Luca Fancellu <luca.fancellu@arm.com>
Acked-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit e11f576650)
Dealing with exceptions raised from within emulation stubs involves
discarding return address (replaced by exception related information).
Such discarding of course also requires removing the corresponding entry
from the shadow stack.
Also amend the comment in fixup_exception_return(), to further clarify
why use of ptr[1] can't be an out-of-bounds access.
This is CVE-2023-46841 / XSA-451.
Fixes: 209fb9919b ("x86/extable: Adjust extable handling to be shadow stack compatible")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 91f5f7a915
master date: 2024-02-27 13:49:22 +0100
The current logic to handle the BRANCH_HARDEN option will report it as enabled
even when build-time disabled. Fix this by only allowing the option to be set
when support for it is built into Xen.
Fixes: 2d6f36daa0 ('x86/nospec: Introduce CONFIG_SPECULATIVE_HARDEN_BRANCH')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 60e00f77a5
master date: 2024-02-26 16:06:42 +0100
The current code for alternative calls uses the caller parameter types as the
types for the register variables that serve as function parameters:
uint8_t foo;
[...]
alternative_call(myfunc, foo);
Would expand roughly into:
register unint8_t a1_ asm("rdi") = foo;
register unsigned long a2_ asm("rsi");
[...]
asm volatile ("call *%c[addr](%%rip)"...);
However with -O2 clang will generate incorrect code, given the following
example:
unsigned int func(uint8_t t)
{
return t;
}
static void bar(uint8_t b)
{
int ret_;
register uint8_t di asm("rdi") = b;
register unsigned long si asm("rsi");
register unsigned long dx asm("rdx");
register unsigned long cx asm("rcx");
register unsigned long r8 asm("r8");
register unsigned long r9 asm("r9");
register unsigned long r10 asm("r10");
register unsigned long r11 asm("r11");
asm volatile ( "call %c[addr]"
: "+r" (di), "=r" (si), "=r" (dx),
"=r" (cx), "=r" (r8), "=r" (r9),
"=r" (r10), "=r" (r11), "=a" (ret_)
: [addr] "i" (&(func)), "g" (func)
: "memory" );
}
void foo(unsigned int a)
{
bar(a);
}
Clang generates the following assembly code:
func: # @func
movl %edi, %eax
retq
foo: # @foo
callq func
retq
Note the truncation of the unsigned int parameter 'a' of foo() to uint8_t when
passed into bar() is lost. clang doesn't zero extend the parameters in the
callee when required, as the psABI mandates.
The above can be worked around by using a union when defining the register
variables, so that `di` becomes:
register union {
uint8_t e;
unsigned long r;
} di asm("rdi") = { .e = b };
Which results in following code generated for `foo()`:
foo: # @foo
movzbl %dil, %edi
callq func
retq
So the truncation is not longer lost. Apply such workaround only when built
with clang.
Reported-by: Matthew Grooms <mgrooms@shrew.net>
Link: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=277200
Link: https://github.com/llvm/llvm-project/issues/12579
Link: https://github.com/llvm/llvm-project/issues/82598
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
master commit: 2ce562b2a4
master date: 2024-02-26 10:18:01 +0100
'*' sets the width field, which is the minimum number of characters to output,
but what we want in no_config_param() is the precision instead, which is '.*'
as it imposes a maximum limit on the output.
Fixes: 68d757df8d ('x86/pv: Options to disable and/or compile out 32bit PV support')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: ef101f5251
master date: 2024-02-26 10:17:40 +0100
The test code has a typo in using LIVEPATCH_APPLY_HOOK() instead of
LIVEPATCH_REVERT_HOOK().
Fixes: 6047104c3c ('livepatch: Add per-function applied/reverted state tracking marker')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
master commit: f0622dd4fd
master date: 2024-02-26 10:13:46 +0100
EVEX.R' is not ignored in 64-bit code when encoding a GPR or mask
register. While for mask registers suitable checks are in place (there
also covering EVEX.R), they were missing for the few cases where in
EVEX-encoded instructions ModR/M.reg encodes a GPR. While for VPEXTRW
the bit is replaced before an emulation stub is invoked, for
VCVT{,T}{S,D,H}2{,U}SI this actually would have led to #UD from inside
an emulation stub, in turn raising #UD to the guest, but accompanied by
log messages indicating something's wrong in Xen nevertheless.
Fixes: 001bd91ad8 ("x86emul: support AVX512{F,BW,DQ} extract insns")
Fixes: baf4a376f5 ("x86emul: support AVX512F legacy-equivalent scalar int/FP conversion insns")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: cb319824bf
master date: 2024-02-22 11:54:07 +0100
Because of using "-include", failure to (re)build auto.conf (with
auto.conf.cmd produced as a secondary target) won't stop make from
continuing the build. Arrange for it being possible to drop the - from
Rules.mk, requiring that the include be skipped for tools-only targets.
Note that relying on the inclusion in those cases wouldn't be correct
anyway, as it might be a stale file (yet to be rebuilt) which would be
included, while during initial build, the file would be absent
altogether.
Fixes: 8d4c17a90b ("xen/build: silence make warnings about missing auto.conf*")
Reported-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
master commit: d34e5fa2e8
master date: 2024-02-22 11:52:47 +0100
According to comments (and experiments) qemu-xen cannot handle memory
reolcation done by hvmloader. The code was already disabled when running
qemu-xen in dom0 (see libxl__spawn_local_dm()), but it was missed when
adding qemu-xen support to stubdomain. Adjust libxl__spawn_stub_dm() to
be consistent in this regard.
Reported-by: Neowutran <xen@neowutran.ovh>
Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
master commit: 97883aa269
master date: 2024-02-22 11:48:22 +0100
The `which` command is not standard, may not exist on the build host,
or may not behave as expected by the build system. It is recommended
to use `command -v` to find out if a command exist and have its path,
and it's part of a POSIX shell standard (at least, it seems to be
mandatory since IEEE Std 1003.1-2008, but was optional before).
Fixes: c8a8645f1e ("xen/build: Automatically locate a suitable python interpreter")
Fixes: 3b47bcdb6d ("xen/build: Use a distro version of figlet")
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Tested-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: f93629b18b
master date: 2024-02-19 12:45:48 +0100
While in the vast majority of cases failure of the function will not
be followed by re-invocation with the same emulation context, a few
very specific insns - involving multiple independent writes, e.g. ENTER
and PUSHA - exist where this can happen. Since failure of the function
only signals to the caller that it ought to try an MMIO write instead,
such failure also cannot be assumed to result in wholesale failure of
emulation of the current insn. Instead we have to maintain internal
state such that another invocation of the function with the same
emulation context remains possible. To achieve that we need to reset MFN
slots after putting page references on the error path.
Note that all of this affects debugging code only, in causing an
assertion to trigger (higher up in the function). There's otherwise no
misbehavior - such a "leftover" slot would simply be overwritten by new
contents in a release build.
Also extend the related unmap() assertion, to further check for MFN 0.
Fixes: 8cbd4fb0b7 ("x86/hvm: implement hvmemul_write() using real mappings")
Reported-by: Manuel Andreas <manuel.andreas@tum.de>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Paul Durrant <paul@xen.org>
master commit: e72f951df4
master date: 2024-02-13 09:36:14 +0100
This patch addresses an issue where the fast singlestep setting would persist
despite xc_domain_debug_control being called with XEN_DOMCTL_DEBUG_OP_SINGLE_STEP_OFF.
Specifically, if fast singlestep was enabled in a VMI session and that session
stopped before the MTF trap occurred, the fast singlestep setting remained
active even though MTF itself was disabled. This led to a situation where, upon
starting a new VMI session, the first event to trigger an EPT violation would
cause the corresponding EPT event callback to be skipped due to the lingering
fast singlestep setting.
The fix ensures that the fast singlestep setting is properly reset when
disabling single step debugging operations.
Signed-off-by: Petr Beneš <w1benny@gmail.com>
Reviewed-by: Tamas K Lengyel <tamas@tklengyel.com>
master commit: 897def94b5
master date: 2024-02-12 09:37:58 +0100
The current code that parses the IVMD blocks is relaxed with regard to the
restriction that such unity regions should always fall into memory ranges
marked as reserved in the memory map.
However the type checks for the IVMD addresses are inverted, and as a result
IVMD ranges falling into RAM areas are accepted. Note that having such ranges
in the first place is a firmware bug, as IVMD should always fall into reserved
ranges.
Fixes: ed6c77ebf0 ('AMD/IOMMU: check / convert IVMD ranges for being / to be reserved')
Reported-by: Ox <oxjo@proton.me>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Tested-by: oxjo <oxjo@proton.me>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 83afa31358
master date: 2024-02-06 11:56:13 +0100
The MMIO RO rangeset overlap check is bogus: the rangeset is inclusive so the
passed end mfn should be the last mfn to be mapped (not last + 1).
Fixes: 6fa1755644 ('amd/npt/shadow: replace assert that prevents creating 2M/1G MMIO entries')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@cloud.com>
master commit: 610775d0dd
master date: 2024-01-25 16:09:04 +0100
At the moment, trying to run xencov read/reset (calling SYSCTL_coverage_op
under the hood) results in a crash. This is due to a profiler trying to
access data in the .init.* sections (libfdt for Arm and libelf for x86)
that are stripped after boot. Normally, the build system compiles any
*.init.o file without COV_FLAGS. However, these two libraries are
handled differently as sections will be renamed to init after linking.
To override COV_FLAGS to empty for these libraries, lib{fdt,elf}.o were
added to nocov-y. This worked until e321576f40 ("xen/build: start using
if_changed") that added lib{fdt,elf}-temp.o and their deps to extra-y.
This way, even though these objects appear as prerequisites of
lib{fdt,elf}.o and the settings should propagate to them, make can also
build them as a prerequisite of __build, in which case COV_FLAGS would
still have the unwanted flags. Fix it by switching to $(targets) instead.
Also, for libfdt, append libfdt.o to nocov-y only if CONFIG_OVERLAY_DTB
is not set. Otherwise, there is no section renaming and we should be able
to run the coverage.
Fixes: e321576f40 ("xen/build: start using if_changed")
Signed-off-by: Michal Orzel <michal.orzel@amd.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
master commit: 79519fcfa0
master date: 2024-01-23 12:02:44 +0100
Right now, vvmx will blindly copy L12's ACTIVITY_STATE into the L02 VMCS and
enter the vCPU. Luckily for us, nested-virt is explicitly unsupported for
security bugs.
The inactivity states are HLT, SHUTDOWN and WAIT-FOR-SIPI, and as noted by the
SDM in Vol3 27.7 "Special Features of VM Entry":
If VM entry ends with the logical processor in an inactive activity state,
the VM entry generates any special bus cycle that is normally generated when
that activity state is entered from the active state.
Also,
Some activity states unconditionally block certain events.
I.e. A VMEntry with ACTIVITY=SHUTDOWN will initiate a platform reset, while a
VMEntry with ACTIVITY=WAIT-FOR-SIPI will really block everything other than
SIPIs.
Both of these activity states are for the TXT ACM to use, not for regular
hypervisors, and Xen doesn't support dropping the HLT intercept either.
There are two paths in Xen which operate on ACTIVITY_STATE.
1) The vmx_{get,set}_nonreg_state() helpers for VM-Fork.
As regular VMs can't use any inactivity states, this is just duplicating
the 0 from construct_vmcs(). Retain the ability to query activity_state,
but crash the domain on any attempt to set an inactivity state.
2) Nested virt, because of ACTIVITY_STATE in vmcs_gstate_field[].
Explicitly hide the inactivity states in the guest's view of MSR_VMX_MISC,
and remove ACTIVITY_STATE from vmcs_gstate_field[].
In virtual_vmentry(), we should trigger a VMEntry failure for the use of
any inactivity states, but there's no support for that in the code at all
so leave a TODO for when we finally start working on nested-virt in
earnest.
Reported-by: Reima Ishii <ishiir@g.ecc.u-tokyo.ac.jp>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Tamas K Lengyel <tamas@tklengyel.com>
master commit: 3643bb53a0
master date: 2024-01-18 20:59:06 +0000
When receiving an INIT, a prior bugfix tried to ignore the INIT and continue
onwards.
Unfortunately it's not safe to return at that point in vmx_vmexit_handler().
Just out of context in the first hunk is a local_irqs_enabled() which is
depended-upon by the return-to-guest path, causing the following checklock
failure in debug builds:
(XEN) Error: INIT received - ignoring
(XEN) CHECKLOCK FAILURE: prev irqsafe: 0, curr irqsafe 1
(XEN) Xen BUG at common/spinlock.c:132
(XEN) ----[ Xen-4.19-unstable x86_64 debug=y Tainted: H ]----
...
(XEN) Xen call trace:
(XEN) [<ffff82d040238e10>] R check_lock+0xcd/0xe1
(XEN) [<ffff82d040238fe3>] F _spin_lock+0x1b/0x60
(XEN) [<ffff82d0402ed6a8>] F pt_update_irq+0x32/0x3bb
(XEN) [<ffff82d0402b9632>] F vmx_intr_assist+0x3b/0x51d
(XEN) [<ffff82d040206447>] F vmx_asm_vmexit_handler+0xf7/0x210
Luckily, this is benign in release builds. Accidentally having IRQs disabled
when trying to take an IRQs-on lock isn't a deadlock-vulnerable pattern.
Drop the problematic early return. In hindsight, it's wrong to skip other
normal VMExit steps.
Fixes: b1f11273d5 ("x86/vmx: Don't spuriously crash the domain when INIT is received")
Reported-by: Reima ISHII <ishiir@g.ecc.u-tokyo.ac.jp>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: d1f8883aeb
master date: 2024-01-18 20:59:06 +0000
When Architectural Performance Monitoring is available, the PERF_GLOBAL_CTRL
MSR contains per-counter enable bits that is ANDed with the enable bit in the
counter EVNTSEL MSR in order for a PMC counter to be enabled.
So far the watchdog code seems to have relied on the PERF_GLOBAL_CTRL enable
bits being set by default, but at least on some Intel Sapphire and Emerald
Rapids this is no longer the case, and Xen reports:
Testing NMI watchdog on all CPUs: 0 40 stuck
The first CPU on each package is started with PERF_GLOBAL_CTRL zeroed, so PMC0
doesn't start counting when the enable bit in EVNTSEL0 is set, due to the
relevant enable bit in PERF_GLOBAL_CTRL not being set.
Check and adjust PERF_GLOBAL_CTRL during CPU initialization so that all the
general-purpose PMCs are enabled. Doing so brings the state of the package-BSP
PERF_GLOBAL_CTRL in line with the rest of the CPUs on the system.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
master commit: 6bdb965178
master date: 2024-01-17 10:40:52 +0100
Jan Beulich
Olaf Hering
Roger Pau Monné
Andrew Cooper
Jason Andryuk
Michal Orzel
Marek Marczykowski-Górecki
Anthony PERARD
Petr Beneš
Cyril Rébert (zithro)