Commit Graph
15897 Commits
Author SHA1 Message Date
Matthew Barnes 8aa5245ef6 tools/lsevtchn: Use errno macro to handle hypercall error cases
Currently, lsevtchn aborts its event channel enumeration when it hits
an event channel that is owned by Xen.

lsevtchn does not distinguish between different hypercall errors, which
results in lsevtchn missing potential relevant event channels with
higher port numbers.

Use the errno macro to distinguish between hypercall errors, and
continue event channel enumeration if the hypercall error is not
critical to enumeration.

Signed-off-by: Matthew Barnes <matthew.barnes@cloud.com>
Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
master commit: e92a453c8d
master date: 2024-08-02 08:43:57 +0200
2024-08-08 13:53:26 +02:00
Andrew Cooper 81f1e807fa tools/libxs: Fix fcntl() invocation in set_cloexec()
set_cloexec() had a bit too much copy&pate from setnonblock(), and
insufficient testing on ancient versions of Linux...

As written (emulating ancient linux by undef'ing O_CLOEXEC), strace shows:

  open("/dev/xen/xenbus", O_RDWR)         = 3
  fcntl(3, F_GETFL)                       = 0x8002 (flags O_RDWR|O_LARGEFILE)
  fcntl(3, 0x8003 /* F_??? */, 0x7ffe4a771d90) = -1 EINVAL (Invalid argument)
  close(3)                                = 0

which is obviously nonsense.

Switch F_GETFL -> F_GETFD, and fix the second invocation to use F_SETFD.  With
this, strace is rather happer:

  open("/dev/xen/xenbus", O_RDWR)         = 3
  fcntl(3, F_GETFD)                       = 0
  fcntl(3, F_SETFD, FD_CLOEXEC)           = 0

Fixes: bf7c146470 ("tools/libxs: Fix CLOEXEC handling in get_dev()")
Reported-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
master commit: 37810b52d0
master date: 2024-07-09 15:32:18 +0100
2024-07-25 16:20:53 +02:00
Andrew Cooper 26b8ff1861 tools/libxs: Fix CLOEXEC handling in xs_fileno()
xs_fileno() opens a pipe on first use to communicate between the watch thread
and the main thread.  Nothing ever sets CLOEXEC on the file descriptors.

Check for the availability of the pipe2() function with configure.  Despite
starting life as Linux-only, FreeBSD and NetBSD have gained it.

When pipe2() isn't available, try our best with pipe() and set_cloexec().

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
master commit: a2ff677852
master date: 2024-07-02 10:52:59 +0100
2024-07-04 14:13:30 +02:00
Andrew Cooper d689bb4d2c tools/libxs: Fix CLOEXEC handling in get_socket()
get_socket() opens a socket, then uses fcntl() to set CLOEXEC.  This is racy
with exec().

Open the socket with SOCK_CLOEXEC.  Use the same compatibility strategy as
O_CLOEXEC on ancient versions of Linux.

Reported-by: Frediano Ziglio <frediano.ziglio@cloud.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
master commit: 1957dd6aff
master date: 2024-07-02 10:51:11 +0100
2024-07-04 14:13:18 +02:00
Andrew Cooper d1b3bbb464 tools/libxs: Fix CLOEXEC handling in get_dev()
Move the O_CLOEXEC compatibility outside of an #ifdef USE_PTHREAD block.

Introduce set_cloexec() to wrap fcntl() setting FD_CLOEXEC.  It will be reused
for other CLOEXEC fixes too.

Use set_cloexec() when O_CLOEXEC isn't available as a best-effort fallback.

Fixes: f4f2f3402b ("tools/libxs: Open /dev/xen/xenbus fds as O_CLOEXEC")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
master commit: bf7c146470
master date: 2024-07-02 10:51:06 +0100
2024-07-04 14:13:10 +02:00
Andrew Cooper 8e51c8f1d4 tools/dombuilder: Correct the length calculation in xc_dom_alloc_segment()
xc_dom_alloc_segment() is passed a size in bytes, calculates a size in pages
from it, then fills in the new segment information with a bytes value
re-calculated from the number of pages.

This causes the module information given to the guest (MB, or PVH) to have
incorrect sizes; specifically, sizes rounded up to the next page.

This in turn is problematic for Xen.  When Xen finds a gzipped module, it
peeks at the end metadata to judge the decompressed size, which is a -4
backreference from the reported end of the module.

Fill in seg->vend using the correct number of bytes.

Fixes: ea7c8a3d0e ("libxc: reorganize domain builder guest memory allocator")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
master commit: 4c3a618b0a
master date: 2024-07-02 10:50:18 +0100
2024-07-04 14:12:31 +02:00
Andrew Cooper 2b3bf02c4f tools/xl: Open xldevd.log with O_CLOEXEC
`xl devd` has been observed leaking /var/log/xldevd.log into children.

Note this is specifically safe; dup2() leaves O_CLOEXEC disabled on newfd, so
after setting up stdout/stderr, it's only the logfile fd which will close on
exec().

Link: https://github.com/QubesOS/qubes-issues/issues/8292
Reported-by: Demi Marie Obenour <demi@invisiblethingslab.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Demi Marie Obenour <demi@invisiblethingslab.com>
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
master commit: ba52b3b624
master date: 2024-06-24 16:22:59 +0100
2024-07-04 14:11:36 +02:00
Andrew Cooper 7967bd358e tools: Drop libsystemd as a dependency
There are no more users, and we want to disuade people from introducing new
users just for sd_notify() and friends.  Drop the dependency.

We still want the overall --with{,out}-systemd to gate the generation of the
service/unit/mount/etc files.

Rerun autogen.sh, and mark the dependency as removed in the build containers.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>

tools: (Actually) drop libsystemd as a dependency

When reinstating some of systemd.m4 between v1 and v2, I reintroduced a little
too much.  While {c,o}xenstored are indeed no longer linked against
libsystemd, ./configure still looks for it.

Drop this too.

Fixes: ae26101f6b ("tools: Drop libsystemd as a dependency")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: ae26101f6b
master date: 2024-05-23 15:04:40 +0100
master commit: 6ef4fa1e7f
master date: 2024-05-30 12:15:25 +0100
2024-07-04 14:10:40 +02:00
Andrew Cooper 77cf215157 tools/{c,o}xenstored: Don't link against libsystemd
Use the local freestanding wrapper instead.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
master commit: caf8644826
master date: 2024-05-23 15:04:40 +0100
2024-07-04 14:10:29 +02:00
Andrew Cooper 1743102a92 tools: Import stand-alone sd_notify() implementation from systemd
... in order to avoid linking against the whole of libsystemd.

Only minimal changes to the upstream copy, to function as a drop-in
replacement for sd_notify() and as a header-only library.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
master commit: 78510f3a15
master date: 2024-05-23 15:04:40 +0100
master commit: 78510f3a15
master date: 2024-05-23 15:04:40 +0100
2024-07-04 14:10:10 +02:00
Juergen Gross 22f623622c tools/tests: let test-xenstore exit with non-0 status in case of error
In case a test is failing in test-xenstore, let the tool exit with an
exit status other than 0.

Fix a typo in an error message.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Fixes: 3afc5e4a5b ("tools/tests: add xenstore testing framework")
Signed-off-by: Juergen Gross <jgross@suse.com>
master commit: 2d4ba20559
master date: 2024-05-02 18:15:46 +0100
2024-07-04 14:07:12 +02:00
Juergen Gross 0ebfa35965 tools/tests: don't let test-xenstore write nodes exceeding default size
Today test-xenstore will write nodes with 3000 bytes node data. This
size is exceeding the default quota for the allowed node size. While
working in dom0 with C-xenstored, OCAML-xenstored does not like that.

Use a size of 2000 instead, which is lower than the allowed default
node size of 2048.

Fixes: 3afc5e4a5b ("tools/tests: add xenstore testing framework")
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 642005e310
master date: 2024-05-02 18:15:31 +0100
2024-07-04 14:06:54 +02:00
Leigh Brown f3d20dd317 tools/xentop: Fix cpu% sort order
In compare_cpu_pct(), there is a double -> unsigned long long converion when
calling compare().  In C, this discards the fractional part, resulting in an
out-of order sorting such as:

        NAME  STATE   CPU(sec) CPU(%)
       xendd --b---       4020    5.7
    icecream --b---       2600    3.8
    Domain-0 -----r       1060    1.5
        neon --b---        827    1.1
      cheese --b---        225    0.7
       pizza --b---        359    0.5
     cassini --b---        490    0.4
     fusilli --b---        159    0.2
         bob --b---        502    0.2
     blender --b---        121    0.2
       bread --b---         69    0.1
    chickpea --b---         67    0.1
      lentil --b---         67    0.1

Introduce compare_dbl() function and update compare_cpu_pct() to call it.

Fixes: 49839b535b ("Add xenstat framework.")
Signed-off-by: Leigh Brown <leigh@solinno.co.uk>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: e27fc7d15e
master date: 2024-05-15 19:59:52 +0100
2024-05-21 10:25:30 +02:00
Demi Marie Obenour 8271f0e8f2 libxl: Fix handling XenStore errors in device creation
If xenstored runs out of memory it is possible for it to fail operations
that should succeed.  libxl wasn't robust against this, and could fail
to ensure that the TTY path of a non-initial console was created and
read-only for guests.  This doesn't qualify for an XSA because guests
should not be able to run xenstored out of memory, but it still needs to
be fixed.

Add the missing error checks to ensure that all errors are properly
handled and that at no point can a guest make the TTY path of its
frontend directory writable.

Signed-off-by: Demi Marie Obenour <demi@invisiblethingslab.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
master commit: 531d3bea5e
master date: 2024-05-11 00:13:43 +0100
2024-05-21 10:23:52 +02:00
Roger Pau Monné 9966e54131 libxl: fix population of the online vCPU bitmap for PVH
libxl passes some information to libacpi to create the ACPI table for a PVH
guest, and among that information it's a bitmap of which vCPUs are online
which can be less than the maximum number of vCPUs assigned to the domain.

While the population of the bitmap is done correctly for HVM based on the
number of online vCPUs, for PVH the population of the bitmap is done based on
the number of maximum vCPUs allowed.  This leads to all local APIC entries in
the MADT being set as enabled, which contradicts the data in xenstore if vCPUs
is different than maximum vCPUs.

Fix by copying the internal libxl bitmap that's populated based on the vCPUs
parameter.

Reported-by: Arthur Borsboom <arthurborsboom@gmail.com>
Link: https://gitlab.com/libvirt/libvirt/-/issues/399
Reported-by: Leigh Brown <leigh@solinno.co.uk>
Fixes: 14c0d328da ('libxl/acpi: Build ACPI tables for HVMlite guests')
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Tested-by: Leigh Brown <leigh@solinno.co.uk>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 5cc7347b04
master date: 2024-05-11 00:13:43 +0100
2024-05-21 10:23:27 +02:00
Andrew Cooper a42c83b202 x86/ucode: Distinguish "ucode already up to date"
Right now, Xen returns -ENOENT for both "the provided blob isn't correct for
this CPU", and "the blob isn't newer than what's loaded".

This in turn causes xen-ucode to exit with an error, when "nothing to do" is
more commonly a success condition.

Handle EEXIST specially and exit cleanly.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: 648db37a15
master date: 2024-05-09 18:19:49 +0100
2024-05-21 10:22:52 +02:00
Andrew Cooper 2bc52041ca tools/libxs: Open /dev/xen/xenbus fds as O_CLOEXEC
The header description for xs_open() goes as far as to suggest that the fd is
O_CLOEXEC, but it isn't actually.

`xl devd` has been observed leaking /dev/xen/xenbus into children.

Link: https://github.com/QubesOS/qubes-issues/issues/8292
Reported-by: Demi Marie Obenour <demi@invisiblethingslab.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
master commit: f4f2f3402b
master date: 2024-05-07 15:18:36 +0100
2024-05-21 10:21:47 +02:00
Jason Andryuk 026542c857 xen/xsm: Wire up get_dom0_console
An XSM hook for get_dom0_console is currently missing.  Using XSM with
a PVH dom0 shows:
(XEN) FLASK: Denying unknown platform_op: 64.

Wire up the hook, and allow it for dom0.

Fixes: 4dd160583c ("x86/platform: introduce hypercall to get initial video console settings")
Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
Acked-by: Daniel P. Smith <dpsmith@apertussolutions.com>
master commit: 647f7e50eb
master date: 2024-04-30 08:33:41 +0200
2024-05-21 10:19:43 +02:00
Andrew Cooper 2be76f2018 x86/tsx: Expose RTM_ALWAYS_ABORT to guests
A TSX Abort is one option mitigate Native-BHI, but a guest kernel doesn't get
to see this if Xen has turned RTM off using MSR_TSX_{CTRL,FORCE_ABORT}.

Therefore, the meaning of RTM_ALWAYS_ABORT has been adjusted to "XBEGIN won't
fault", and it should be exposed to guests so they can make a better decision.

Expose it in the max policy for any RTM-capable system.  Offer it by default
only if RTM has been disabled.

Update test-tsx to account for this new meaning.  While adjusting the logic in
test_guest_policies(), take the opportunity to use feature names (now they're
available) to make the logic easier to follow.

This is part of XSA-456 / CVE-2024-2201.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit c94e210592)
2024-04-09 16:45:01 +01:00
Andrew Cooper 17cf285d87 tests/resource: Fix HVM guest in !SHADOW builds
Right now, test-resource always creates HVM Shadow guests.  But if Xen has
SHADOW compiled out, running the test yields:

  $./test-resource
  XENMEM_acquire_resource tests
  Test x86 PV
    Created d1
    Test grant table
  Test x86 PVH
    Skip: 95 - Operation not supported

and doesn't really test HVM guests, but doesn't fail either.

There's nothing paging-mode-specific about this test, so default to HAP if
possible and provide a more specific message if neither HAP or Shadow are
available.

As we've got physinfo to hand, also provide more specific message about the
absence of PV or HVM support.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
master commit: 0263dc9069
master date: 2024-03-01 20:14:19 +0000
2024-04-02 16:23:54 +02:00
Edwin Török 0a8b92d0a4 tools/oxenstored: Make Quota.t pure
Now that we no longer have a hashtable inside we can make Quota.t pure, and
push the mutable update to its callers.  Store.t already had a mutable Quota.t
field.

No functional change.

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
Acked-by: Christian Lindig <christian.lindig@cloud.com>
(cherry picked from commit 098d868e52)
2024-03-27 16:06:20 +00:00
Edwin Török 3f3158fc32 tools/oxenstored: Use Map instead of Hashtbl for quotas
On a stress test running 1000 VMs flamegraphs have shown that
`oxenstored` spends a large amount of time in `Hashtbl.copy` and the GC.

Hashtable complexity:
 * read/write: O(1) average
 * copy: O(domains) -- copying the entire table

Map complexity:
 * read/write: O(log n) worst case
 * copy: O(1) -- a word copy

We always perform at least one 'copy' when processing each xenstore
packet (regardless whether it is a readonly operation or inside a
transaction or not), so the actual complexity per packet is:
  * Hashtbl: O(domains)
  * Map: O(log domains)

Maps are the clear winner, and a better fit for the immutable xenstore
tree.

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

tools/oxenstored: Re-format

Rerun make format.

Fixes: b6cf604207 ("tools/oxenstored: Use Map instead of Hashtbl for quotas")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Edwin Török <edwin.torok@cloud.com>
(cherry picked from commit 8f85af65af)
2024-03-27 16:05:30 +00:00
Andrew Cooper 908cbd1893 x86/spec-ctrl: Mitigation Register File Data Sampling
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)
2024-03-12 15:55:52 +00:00
Jan Beulich 4c84fa6cb6 hvmloader/PCI: skip huge BARs in certain calculations
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
2024-03-12 12:06:57 +01:00
Jason Andryuk 267845a838 libxl: Fix segfault in device_model_spawn_outcome
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
2024-03-05 11:54:12 +01:00
Marek Marczykowski-Górecki 489c2b9ba1 libxl: Disable relocating memory for qemu-xen in stubdomain too
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
2024-02-27 13:57:07 +01:00
Jason Andryuk 16475909ba block-common: Fix same_vm for no targets
same_vm is broken when the two main domains do not have targets.  otvm
and targetvm are both missing, which means they get set to -1 and then
converted to empty strings:

++10697+ local targetvm=-1
++10697+ local otvm=-1
++10697+ otvm=
++10697+ othervm=/vm/cc97bc2f-3a91-43f7-8fbc-4cb92f90b4e4
++10697+ targetvm=
++10697+ local frontend_uuid=/vm/844dea4e-44f8-4e3e-8145-325132a31ca5

The final comparison returns true since the two empty strings match:

++10697+ '[' /vm/844dea4e-44f8-4e3e-8145-325132a31ca5 = /vm/cc97bc2f-3a91-43f7-8fbc-4cb92f90b4e4 -o '' = /vm/cc97bc2f-3a91-43f7-8fbc-4cb92f90b4e4 -o /vm/844dea4e-44f8-4e3e-8145-325132a31ca5 = '' -o '' = '' ']'

Replace -1 with distinct strings indicating the lack of a value and
remove the collescing to empty stings.  The strings themselves will no
longer match, and that is correct.

++12364+ '[' /vm/844dea4e-44f8-4e3e-8145-325132a31ca5 = /vm/cc97bc2f-3a91-43f7-8fbc-4cb92f90b4e4 -o 'No target' = /vm/cc97bc2f-3a91-43f7-8fbc-4cb92f90b4e4 -o /vm/844dea4e-44f8-4e3e-8145-325132a31ca5 = 'No other target' -o 'No target' = 'No other target' ']'

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
master commit: e8f1bb803f
master date: 2024-02-07 13:46:52 +0100
2024-02-27 13:55:03 +01:00
Cyril Rébert (zithro) 184d723e7a tools/xentop: fix sorting bug for some columns
Sort doesn't work on columns VBD_OO, VBD_RD, VBD_WR and VBD_RSECT.
Fix by adjusting variables names in compare functions.
Bug fix only. No functional change.

Fixes: 91c3e3dc91 ("tools/xentop: Display '-' when stats are not available.")
Signed-off-by: Cyril Rébert (zithro) <slack@rabbit.lu>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
master commit: 29f17d8374
master date: 2024-02-05 17:58:23 +0000
2024-02-27 13:53:42 +01:00
Julien Grall a56d598e13 Only compile the hypervisor with -Wdeclaration-after-statement
Right now, all tools and hypervisor will be complied with the option
-Wdeclaration-after-statement. While most of the code in the hypervisor
is controlled by us, for tools we may import external libraries.

The build will fail if one of them are using the construct we are
trying to prevent. This is the case when building against Python 3.12
and Yocto:

| In file included from /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/Python.h:44,
|                  from xen/lowlevel/xc/xc.c:8:
| /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/object.h: In function 'Py_SIZE':
| /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/object.h:233:5: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
|   233 |     PyVarObject *var_ob = _PyVarObject_CAST(ob);
|       |     ^~~~~~~~~~~
| In file included from /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/Python.h:53:
| /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/cpython/longintrepr.h: In function '_PyLong_CompactValue':
| /srv/storage/alex/yocto/build-virt/tmp/work/core2-64-poky-linux/xen-tools/4.17+stable/recipe-sysroot/usr/include/python3.12/cpython/longintrepr.h:121:5: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
|   121 |     Py_ssize_t sign = 1 - (op->long_value.lv_tag & _PyLong_SIGN_MASK);
|       |     ^~~~~~~~~~
| cc1: all warnings being treated as errors

Looking at the tools directory, a fair few directory already add
-Wno-declaration-after-statement to inhibit the default behavior.

We have always build the hypervisor with the flag, so for now remove
only the flag for anything but the hypervisor. We can decide at later
time whether we want to relax.

Also remove the -Wno-declaration-after-statement in some subdirectory
as the flag is now unnecessary.

Part of the commit message was take from Alexander's first proposal:

Link: https://lore.kernel.org/xen-devel/20231128174729.3880113-1-alex@linutronix.de/
Reported-by: Alexander Kanavin <alex@linutronix.de>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Jason Andryuk <jandryuk@gmail.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>

xen/hypervisor: Don't use cc-option-add for -Wdeclaration-after-statement

Per Andrew's comment in [1] all the compilers we support should
recognize the flag.

I forgot to address the comment while committing.

[1] fcf00090-304a-49f7-8a61-a54347e90a3b@citrix.com

Signed-off-by: Julien Grall <jgrall@amazon.com>
master commit: 40be6307ec
master date: 2023-12-06 19:12:40 +0000
master commit: d4bfd38998
master date: 2023-12-06 19:19:59 +0000
2023-12-12 14:26:18 +01:00
Alejandro Vallejo 3af9d1cbb6 tools/xg: Fix potential memory leak in cpu policy getters/setters
They allocate two different hypercall buffers, but leak the first
allocation if the second one failed due to an early return that bypasses
cleanup.

Remove the early exit and go through _post() instead. Invoking _post() is
benign even if _pre() failed.

Fixes: 6b85e42709 ('x86/sysctl: Implement XEN_SYSCTL_get_cpu_policy')
Fixes: 60529dfeca ('x86/domctl: Implement XEN_DOMCTL_get_cpu_policy')
Fixes: 14ba07e6f8 ('x86/domctl: Implement XEN_DOMCTL_set_cpumsr_policy')
Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
master commit: 1571ff7a98
master date: 2023-12-01 10:53:07 +0100
2023-12-06 10:39:55 +01:00
Jason Andryuk 3e12149eb2 golang: Fix bindings after XSA-443
The new bootloader_restrict and bootloader_user fields in the libxl idl
change the bindings.  Update them.

Fixes: 1f762642d2 ("libxl: add support for running bootloader in restricted mode")
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: George Dunlap <george.dunlap@cloud.com>
(cherry picked from commit 1f849edc2f)
2023-11-07 09:59:06 +00:00
Jason Andryuk 51b8bbe580 golang: Fixup binding for Arm FF-A
The new FF-A TEE type changed the go bindings.  Update them.

Fixes: 8abdd8d528 ("tools: add Arm FF-A mediator")
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: George Dunlap <george.dunlap@cloud.com>
(cherry picked from commit 1429f9c548)
2023-11-07 09:59:06 +00:00
Julien Grall b6cf4f81b5 Turn off debug by default
Signed-off-by: Julien Grall <jgrall@amazon.com>
2023-10-27 14:08:16 +01:00
Alejandro Vallejo 40387f6206 tools/pygrub: Fix pygrub's --entry flag for python3
string.atoi() has been deprecated since Python 2.0, has a big scary warning
in the python2.7 docs and is absent from python3 altogether. int() does the
same thing and is compatible with both.

See https://docs.python.org/2/library/string.html#string.atoi:

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2023-10-18 15:44:31 +01:00
George Dunlap e58bd71c09 cxenstored: wait until after reset to notify dom0less domains
Commit fc2b57c9a ("xenstored: send an evtchn notification on
introduce_domain") introduced the sending of an event channel to the
guest when first introduced, so that dom0less domains waiting for the
connection would know that xenstore was ready to use.

Unfortunately, it was introduced in introduce_domain(), which 1) is
called by other functions, where such functionality is unneeded, and
2) after the main XS_INTRODUCE call, calls domain_conn_reset().  This
introduces a race condition, whereby if xenstored is delayed, a domain
can wake up, send messages to the buffer, only to have them deleted by
xenstore before finishing its processing of the XS_INTRODUCE message.

Move the connect-and-notfy call into do_introduce() instead, after the
domain_conn_rest(); predicated on the state being in the
XENSTORE_RECONNECT state.

(We don't need to check for "restoring", since that value is always
passed as "false" from do_domain_introduce()).

Also take the opportunity to add a missing wmb barrier after resetting
the indexes of the ring in domain_conn_reset.

This change will also remove an extra event channel notification for
dom0 (because the notification is now done by do_introduce which is not
called for dom0.) The extra dom0 event channel notification was only
introduced by fc2b57c9a and was never present before. It is not needed
because dom0 is the one to tell xenstored the connection parameters, so
dom0 has to know that the ring page is setup correctly by the time
xenstored starts looking at it. It is dom0 that performs the ring page
init.

Signed-off-by: George Dunlap <george.dunlap@cloud.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
CC: jgross@suse.com
CC: julien@xen.org
CC: wl@xen.org
2023-10-17 13:56:55 -07:00
George Dunlap 0ce2ee7a16 xenalyze: Reduce warnings about leaving a vcpu in INIT
We warn when we see data for a vcpu moving into a non-RUNNING state,
just so that people know why we're ignoring it.  On full traces, this
happens only once.  However, if the trace was limited to a subset of
pcpus, then this will happen every time the domain in question is
woken on that pcpu.

Add a 'delayed_init' flag to the vcpu struct to indicate when a vcpu
has experienced a delayed init.  Print a warning message once when
entering the state, and once when leaving it.

Signed-off-by: George Dunlap <george.dunlap@cloud.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2023-10-16 15:01:52 +01:00
George Dunlap b26a234135 xenalyze: Fix interrupt EIP reporting
EIP lists are generalized across several use cases.  For many of them,
it make sense to have a cycle per sample; but not really for interrupt
EIP lists.  For this reason, it normally just passes 0 as for the tsc
value, which will in turn down at the bottom of update_cycles(),
update only the summary.event_count, but nothing else.

The dump_eip() function attempted to handle this by calling the generic
cycle print handler if the summary contained *any* cycles, and by collecting
and printing its own stats, based solely on counts, if not.

Unfortunately, it used the wrong element for this: it collected the
total from samples.count rather samples.event_count; in the case that
there are no cycles, this will always be zero.  It then divided by
this zero value.  This results in output that looked like this:

```
  ffff89d29656                                             :        0  -nan%
  ffff89d298b6                                             :        0  -nan%
  ffff89d298c0                                             :        0  -nan%
```

It's better than nothing, but a lot less informative than one would
like.

Use event_count rather than count for collecting the total, and the
reporting when there are no cycles in the summary information.  This results
in output that looks like this:

```
   ffff89d29656                                             :        2  1.21%
   ffff89d298b6                                             :        1  0.61%
   ffff89d298c0                                             :        1  0.61%
```

Signed-off-by: George Dunlap <george.dunlap@cloud.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2023-10-16 15:01:51 +01:00
George Dunlap 4292c54545 xenalyze: Don't expect an HVM_HANDLER trace for PAUSE vmexits
Neither vmx nor svm trace anything, nor is there anything obvious
worth tracing.

Signed-off-by: George Dunlap <george.dunlap@cloud.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2023-10-16 15:01:50 +01:00
George Dunlap 4ef752733b xenalyze: AMD's VMEXIT_VINTR doesn't need a trace record
Just like Intel's PENDING_VIRT_INTR, AMD's VINTR doesn't need an HVM
trace record.  Expect that.

Signed-off-by: George Dunlap <george.dunlap@cloud.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2023-10-16 15:01:48 +01:00
George Dunlap ea6f4cd304 xenalyze: Only accumulate data from one vmexit without a handler
If a vmentry/exit arc unexpectedly doesn't have a handler, we throw an
error, and then log the information under HVM event 0; thus those
particular traces within the vmexit reason will have stats gathered,
and will show up with "(no handler)".  This is useful in the event
that there are unusual paths through the hypervisor which don't have
trace points.

However, if there are more than one of these, then although we detect and warn
that this is happening, we subsequently continue to stuff data about all exits
into that one exit, even though we only show it in one place.

Instead, refator things to only allow a single exit reason to be
accumulated into any given event.

Also put a comment explaining what's going on, and how to fix it.

Signed-off-by: George Dunlap <george.dunlap@cloud.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2023-10-16 15:01:47 +01:00
Roger Pau Monne 9c114178ff libxl: limit bootloader execution in restricted mode
Introduce a timeout for bootloader execution when running in restricted mode.

Allow overwriting the default time out with an environment provided value.

This is part of XSA-443 / CVE-2023-34325

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2023-10-11 06:36:50 +01:00
Roger Pau Monne 1f762642d2 libxl: add support for running bootloader in restricted mode
Much like the device model depriv mode, add the same kind of support for the
bootloader.  Such feature allows passing a UID as a parameter for the
bootloader to run as, together with the bootloader itself taking the necessary
actions to isolate.

Note that the user to run the bootloader as must have the right permissions to
access the guest disk image (in read mode only), and that the bootloader will
be run in non-interactive mode when restricted.

If enabled bootloader restrict mode will attempt to re-use the user(s) from the
QEMU depriv implementation if no user is provided on the configuration file or
the environment.  See docs/features/qemu-deprivilege.pandoc for more
information about how to setup those users.

Bootloader restrict mode is not enabled by default as it requires certain
setup to be done first (setup of the user(s) to use in restrict mode).

This is part of XSA-443 / CVE-2023-34325

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2023-10-11 06:36:50 +01:00
Alejandro Vallejo e0342ae555 tools/pygrub: Deprivilege pygrub
Introduce a --runas=<uid> flag to deprivilege pygrub on Linux and *BSDs. It
also implicitly creates a chroot env where it drops a deprivileged forked
process. The chroot itself is cleaned up at the end.

If the --runas arg is present, then pygrub forks, leaving the child to
deprivilege itself, and waiting for it to complete. When the child exists,
the parent performs cleanup and exits with the same error code.

This is roughly what the child does:
  1. Initialize libfsimage (this loads every .so in memory so the chroot
     can avoid bind-mounting /{,usr}/lib*
  2. Create a temporary empty chroot directory
  3. Mount tmpfs in it
  4. Bind mount the disk inside, because libfsimage expects a path, not a
     file descriptor.
  5. Remount the root tmpfs to be stricter (ro,nosuid,nodev)
  6. Set RLIMIT_FSIZE to a sensibly high amount (128 MiB)
  7. Depriv gid, groups and uid

With this scheme in place, the "output" files are writable (up to
RLIMIT_FSIZE octets) and the exposed filesystem is immutable and contains
the single only file we can't easily get rid of (the disk).

If running on Linux, the child process also unshares mount, IPC, and
network namespaces before dropping its privileges.

This is part of XSA-443 / CVE-2023-34325

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
2023-10-11 06:36:50 +01:00
Alejandro Vallejo 990e65c3ad tools/libfsimage: Export a new function to preload all plugins
This is work required in order to let pygrub operate in highly deprivileged
chroot mode. This patch adds a function that preloads every plugin, hence
ensuring that a on function exit, every shared library is loaded in memory.

The new "init" function is supposed to be used before depriv, but that's
fine because it's not acting on untrusted data.

This is part of XSA-443 / CVE-2023-34325

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
2023-10-11 06:36:50 +01:00
Alejandro Vallejo 0710d7d445 tools/pygrub: Open the output files earlier
This patch allows pygrub to get ahold of every RW file descriptor it needs
early on. A later patch will clamp the filesystem it can access so it can't
obtain any others.

This is part of XSA-443 / CVE-2023-34325

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
2023-10-11 06:36:50 +01:00
Alejandro Vallejo 9f2ff9a7c9 tools/pygrub: Small refactors
Small tidy up to ensure output_directory always has a trailing '/' to ease
concatenating paths and that `output` can only be a filename or None.

This is part of XSA-443 / CVE-2023-34325

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
2023-10-11 06:36:50 +01:00
Alejandro Vallejo f4b504c617 tools/pygrub: Remove unnecessary hypercall
There's a hypercall being issued in order to determine whether PV64 is
supported, but since Xen 4.3 that's strictly true so it's not required.

Plus, this way we can avoid mapping the privcmd interface altogether in the
depriv pygrub.

This is part of XSA-443 / CVE-2023-34325

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2023-10-11 06:36:50 +01:00
Alejandro Vallejo 7d85c70431 libfsimage/xfs: Add compile-time check to libfsimage
Adds the common tools include folder to the -I compile flags
of libfsimage. This allows us to use:
  xen-tools/common-macros.h:BUILD_BUG_ON()

With it, statically assert a sanitized "blocklog - SECTOR_BITS" cannot
underflow.

This is part of XSA-443 / CVE-2023-34325

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-10-11 06:36:50 +01:00
Alejandro Vallejo 620500dd1b libfsimage/xfs: Sanity-check the superblock during mounts
Sanity-check the XFS superblock for wellformedness at the mount handler.
This forces pygrub to abort parsing a potentially malformed filesystem and
ensures the invariants assumed throughout the rest of the code hold.

Also, derive parameters from previously sanitized parameters where possible
(rather than reading them off the superblock)

The code doesn't try to avoid overflowing the end of the disk, because
that's an unlikely and benign error. Parameters used in calculations of
xfs_daddr_t (like the root inode index) aren't in critical need of being
sanitized.

The sanitization of agblklog is basically checking that no obvious
overflows happen on agblklog, and then ensuring agblocks is contained in
the range (2^(sb_agblklog-1), 2^sb_agblklog].

This is part of XSA-443 / CVE-2023-34325

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2023-10-11 06:36:50 +01:00
Alejandro Vallejo ddc45e4eea libfsimage/xfs: Amend mask32lo() to allow the value 32
agblklog could plausibly be 32, but that would overflow this shift.
Perform the shift as ULL and cast to u32 at the end instead.

This is part of XSA-443 / CVE-2023-34325

Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2023-10-11 06:36:49 +01:00