Commit Graph
55 Commits
Author SHA1 Message Date
Jason Andryuk 36e49fc8cb CHANGELOG: mention xl/libxl SMBIOS support
Add an entry for the new xl/libxl SMBIOS support.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Henry Wang <Henry.Wang@arm.com>
2023-03-16 14:50:08 +01:00
Jason Andryuk fc750656aa libxl: Fix libxl__device_pci_reset error messages
Don't use the LOG*D macros.  They expect a domid, but "domain" here is
the PCI domain.  Hence it is inappropriate for this use.

Make the write error messages uniform with LOGE.  errno has the
interesting information while rc is just -1.  Drop printing rc and use
LOGE to print errno as text.

The interesting part of a failed write to do_flr is that PCI BDF, so
print that.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2023-03-15 18:41:46 +00:00
Jason Andryuk 38bdfa44cc xl/libxl: Add OEM string support to smbios
Add support for OEM strings in the SMBIOS type 11.

hvmloader checks them sequentially, so hide the implementation detail.
Allow multiple plain oem= items and assign the numeric values
internally.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2023-03-15 18:41:46 +00:00
Jason Andryuk 9aeb3a20c4 xl/libxl: Add ability to specify SMBIOS strings
hvm_xs_strings.h specifies xenstore entries which can be used to set or
override smbios strings.  hvmloader has support for reading them, but
xl/libxl support is not wired up.

Allow specifying the strings with the new xl.cfg option:
smbios=["bios_vendor=Xen Project","system_version=1.0"]

In terms of strings, the SMBIOS specification 3.5 says:
https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.5.0.pdf
"""
Strings must be encoded as UTF-8 with no byte order mark (BOM). For
compatibility with older SMBIOS parsers, US-ASCII characters should be
used.  NOTE There is no limit on the length of each individual text
string. However, the length of the entire structure table (including all
strings) must be reported in the Structure Table Length field of the
32-bit Structure Table Entry Point (see 5.2.1) and/or the Structure
Table Maximum Size field of the 64-bit Structure Table Entry Point (see
5.2.2).
"""

The strings aren't checked for utf-8 or length.  hvmloader has a sanity
check on the overall length.

The libxl_smbios_type enum starts at 1 since otherwise the 0th key is
not printed in the json output.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2023-03-15 18:41:46 +00:00
Jason Andryuk d03246206c golang/xenlight: Extend KeyedUnion to support Arrays
Generation for KeyedUnion types doesn't support Arrays.  The smbios
support will place an smbios array inside the hvm KeyedUnion, and
gentotypes doesn't generate buildable Go code.

Have KeyedUnion add an idl.Array check and issue the approriate
xenlight_golang_array_to_C and xenlight_golang_array_from_C calls when
needed.  This matches how it is done in xenlight_golang_define_to_C &
xenlight_golang_define_from_C

xenlight_golang_array_to_C and xenlight_golang_array_from_C need to be
extended to set the cvarname and govarname as approriate for the
KeyedUnion cases to match the surrounding code.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
2023-03-15 18:41:46 +00:00
Jason Andryuk de819c96c8 flask/label-pci: Allow specifying optional irq label
IRQs can be shared between devices, so using the same label as the PCI
device can create conflicts where the IRQ is labeled with one of the
device labels preventing assignment of the second device to the second
domain.  Add the ability to specify an irq label distinct from the PCI
device, so a shared irq label can be specified.  The policy would then
be written such that the two domains can each use the shared IRQ type in
addition to their labeled PCI device.  That way we can still label most
of the PCI device resources and assign devices in the face of shared
IRQs.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
2023-03-14 10:46:00 +01:00
Jason Andryuk 1e454c2b5b libxl: fix guest kexec - skip cpuid policy
When a domain performs a kexec (soft reset), libxl__build_pre() is
called with the existing domid.  Calling libxl__cpuid_legacy() on the
existing domain fails since the cpuid policy has already been set, and
the guest isn't rebuilt and doesn't kexec.

xc: error: Failed to set d1's policy (err leaf 0xffffffff, subleaf 0xffffffff, msr 0xffffffff) (17 = File exists): Internal error
libxl: error: libxl_cpuid.c:494:libxl__cpuid_legacy: Domain 1:Failed to apply CPUID policy: File exists
libxl: error: libxl_create.c:1641:domcreate_rebuild_done: Domain 1:cannot (re-)build domain: -3
libxl: error: libxl_xshelp.c:201:libxl__xs_read_mandatory: xenstore read failed: `/libxl/1/type': No such file or directory
libxl: warning: libxl_dom.c:49:libxl__domain_type: unable to get domain type for domid=1, assuming HVM

During a soft_reset, skip calling libxl__cpuid_legacy() to avoid the
issue.  Before commit 34990446ca, the libxl__cpuid_legacy() failure
would have been ignored, so kexec would continue.

Fixes: 34990446ca ("libxl: don't ignore the return value from xc_cpuid_apply_policy")
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2023-01-26 10:58:23 +01:00
Jason Andryuk 10ced96ce9 Revert "tools/xenstore: simplify loop handling connection I/O"
I'm observing guest kexec trigger xenstored to abort on a double free.

gdb output:
Program received signal SIGABRT, Aborted.
__pthread_kill_implementation (no_tid=0, signo=6, threadid=140645614258112) at ./nptl/pthread_kill.c:44
44    ./nptl/pthread_kill.c: No such file or directory.
(gdb) bt
    at ./nptl/pthread_kill.c:44
    at ./nptl/pthread_kill.c:78
    at ./nptl/pthread_kill.c:89
    at ../sysdeps/posix/raise.c:26
    at talloc.c:119
    ptr=ptr@entry=0x559fae724290) at talloc.c:232
    at xenstored_core.c:2945
(gdb) frame 5
    at talloc.c:119
119            TALLOC_ABORT("Bad talloc magic value - double free");
(gdb) frame 7
    at xenstored_core.c:2945
2945                talloc_increase_ref_count(conn);
(gdb) p conn
$1 = (struct connection *) 0x559fae724290

Looking at a xenstore trace, we have:
IN 0x559fae71f250 20230120 17:40:53 READ (/local/domain/3/image/device-model-dom
id )
wrl: dom    0      1  msec      10000 credit     1000000 reserve        100 disc
ard
wrl: dom    3      1  msec      10000 credit     1000000 reserve        100 disc
ard
wrl: dom    0      0  msec      10000 credit     1000000 reserve          0 disc
ard
wrl: dom    3      0  msec      10000 credit     1000000 reserve          0 disc
ard
OUT 0x559fae71f250 20230120 17:40:53 ERROR (ENOENT )
wrl: dom    0      1  msec      10000 credit     1000000 reserve        100 disc
ard
wrl: dom    3      1  msec      10000 credit     1000000 reserve        100 disc
ard
IN 0x559fae71f250 20230120 17:40:53 RELEASE (3 )
DESTROY watch 0x559fae73f630
DESTROY watch 0x559fae75ddf0
DESTROY watch 0x559fae75ec30
DESTROY watch 0x559fae75ea60
DESTROY watch 0x559fae732c00
DESTROY watch 0x559fae72cea0
DESTROY watch 0x559fae728fc0
DESTROY watch 0x559fae729570
DESTROY connection 0x559fae724290
orphaned node /local/domain/3/device/suspend/event-channel deleted
orphaned node /local/domain/3/device/vbd/51712 deleted
orphaned node /local/domain/3/device/vkbd/0 deleted
orphaned node /local/domain/3/device/vif/0 deleted
orphaned node /local/domain/3/control/shutdown deleted
orphaned node /local/domain/3/control/feature-poweroff deleted
orphaned node /local/domain/3/control/feature-reboot deleted
orphaned node /local/domain/3/control/feature-suspend deleted
orphaned node /local/domain/3/control/feature-s3 deleted
orphaned node /local/domain/3/control/feature-s4 deleted
orphaned node /local/domain/3/control/sysrq deleted
orphaned node /local/domain/3/data deleted
orphaned node /local/domain/3/drivers deleted
orphaned node /local/domain/3/feature deleted
orphaned node /local/domain/3/attr deleted
orphaned node /local/domain/3/error deleted
orphaned node /local/domain/3/console/backend-id deleted

and no further output.

The trace shows that DESTROY was called for connection 0x559fae724290,
but that is the same pointer (conn) main() was looping through from
connections.  So it wasn't actually removed from the connections list?

Reverting commit e8e6e42279 "tools/xenstore: simplify loop handling
connection I/O" fixes the abort/double free.  I think the use of
list_for_each_entry_safe is incorrect.  list_for_each_entry_safe makes
traversal safe for deleting the current iterator, but RELEASE/do_release
will delete some other entry in the connections list.  I think the
observed abort is because list_for_each_entry has next pointing to the
deleted connection, and it is used in the subsequent iteration.

Add a comment explaining the unsuitability of list_for_each_entry_safe.
Also notice that the old code takes a reference on next which would
prevents a use-after-free.

This reverts commit e8e6e42279.

This is XSA-425/CVE-2022-42330.

Fixes: e8e6e42279 ("tools/xenstore: simplify loop handling connection I/O")
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
2023-01-26 10:57:37 +01:00
Jason Andryuk 197f612b77 argo: Remove reachable ASSERT_UNREACHABLE
I observed this ASSERT_UNREACHABLE in partner_rings_remove consistently
trip.  It was in OpenXT with the viptables patch applied.

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

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

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Christopher Clark <christopher.w.clark@gmail.com>
Release-acked-by: Henry Wang <Henry.Wang@arm.com>
2022-10-14 14:45:41 +01:00
Jason Andryuk e83cd54611 x86: Expose more MSR_ARCH_CAPS to hwdom
commit e46474278a ("x86/intel: Expose MSR_ARCH_CAPS to dom0") started
exposing MSR_ARCH_CAPS to dom0.  More bits in MSR_ARCH_CAPS have since
been defined, but they haven't been exposed.  Update the list to allow
them through.

As one example, this allows a Linux Dom0 to know that it has the
appropriate microcode via FB_CLEAR.  Notably, and with the updated
microcode, this changes dom0's
/sys/devices/system/cpu/vulnerabilities/mmio_stale_data changes from:

  "Vulnerable: Clear CPU buffers attempted, no microcode; SMT Host state unknown"

to:

  "Mitigation: Clear CPU buffers; SMT Host state unknown"

This exposes the MMIO Stale Data and Intel Branch History Injection
(BHI) controls as well as the page size change MCE issue bit.

Fixes: commit 2ebe8fe9b7 ("x86/spec-ctrl: Enumeration for MMIO Stale Data controls")
Fixes: commit cea9ae0622 ("x86/spec-ctrl: Enumeration for new Intel BHI controls")
Fixes: commit 59e89cdabc ("x86/vtx: Disable executable EPT superpages to work around CVE-2018-12207")
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2022-08-09 16:35:25 +01:00
Jason Andryuk 7fbebfa540 libxl: don't leak self pipes
libxl is leaking self pipes to child processes.  These can be seen when
running with env var _LIBXL_DEBUG_EXEC_FDS=1:

libxl: debug: libxl_aoutils.c:593:libxl__async_exec_start: forking to execute: /etc/xen/scripts/vif-bridge online
[Detaching after fork from child process 5099]
libxl: execing /etc/xen/scripts/vif-bridge: fd 4 is open to pipe:[46805] with flags 0
libxl: execing /etc/xen/scripts/vif-bridge: fd 13 is open to pipe:[46807] with flags 0
libxl: execing /etc/xen/scripts/vif-bridge: fd 14 is open to pipe:[46807] with flags 0
libxl: execing /etc/xen/scripts/vif-bridge: fd 19 is open to pipe:[48570] with flags 0
libxl: execing /etc/xen/scripts/vif-bridge: fd 20 is open to pipe:[48570] with flags 0

(fd 3 is also open, but the check only starts at 4 for some reason.)

For xl, this is the poller created by libxl_ctx_alloc, the poller
created by do_domain_create -> libxl__ao_create, and the self pipe for
libxl__sigchld_needed.  Set CLOEXEC on the FDs so they are not leaked
into children.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2022-06-01 09:18:49 +02:00
Jason Andryuk 0f4fd6b6d8 libxl: retry QMP PCI device_add
PCI device assignment to an HVM with stubdom is potentially racy.  First
the PCI device is assigned to the stubdom via the PV PCI protocol.  Then
QEMU is sent a QMP command to attach the PCI device to QEMU running
within the stubdom.  However, the sysfs entries within the stubdom may
not have appeared by the time QEMU receives the device_add command
resulting in errors like:

libxl_qmp.c:1838:qmp_ev_parse_error_messages:Domain 10:Could not open '/sys/bus/pci/devices/0000:00:1f.3/config': No such file or directory

This patch retries the device assignment up to 10 times with a 1 second
delay between.  That roughly matches the overall hotplug timeout for
pci_add_timeout.  pci_add_timeout's initialization is moved to
do_pci_add since retries call into pci_add_qmp_device_add again.

The qmp_ev_parse_error_messages error is still printed since it happens
at a lower level than the pci code controlling the retries.  With that,
the "Retrying PCI add %d" message is also printed at ERROR level to
clarify what is happening.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2022-04-27 09:14:30 +02:00
Jason Andryuk 2e6f95a942 x86/irq: skip unmap_domain_pirq XSM during destruction
xsm_unmap_domain_irq was seen denying unmap_domain_pirq when called from
complete_domain_destroy as an RCU callback.  The source context was an
unexpected, random domain.  Since this is a xen-internal operation,
going through the XSM hook is inapproriate.

Check d->is_dying and skip the XSM hook when set since this is a cleanup
operation for a domain being destroyed.

Suggested-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
2022-04-08 14:51:52 +02:00
Jason Andryuk d62a34423a libxl: Re-scope qmp_proxy_spawn.ao usage
I've observed this failed assertion:
libxl_event.c:2057: libxl__ao_inprogress_gc: Assertion `ao' failed.

AFAICT, this is happening in qmp_proxy_spawn_outcome where
sdss->qmp_proxy_spawn.ao is NULL.

The out label of spawn_stub_launch_dm() calls qmp_proxy_spawn_outcome(),
but it is only in the success path that sdss->qmp_proxy_spawn.ao gets
set to the current ao.

qmp_proxy_spawn_outcome() should instead use sdss->dm.spawn.ao, which is
the already in-use ao when spawn_stub_launch_dm() is called.  The same
is true for spawn_qmp_proxy().

With this, move sdss->qmp_proxy_spawn.ao initialization to
spawn_qmp_proxy() since its use is for libxl__spawn_spawn() and it can
be initialized along with the rest of sdss->qmp_proxy_spawn.

Fixes: 83c845033d ("libxl: use vchan for QMP access with Linux stubdomain")
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2022-04-01 17:01:57 +01:00
Jason Andryuk d2ecf97f91 libxl: Don't segfault on soft-reset failure
If domain_soft_reset_cb can't rename the save file, it doesn't call
initiate_domain_create() and calls domcreate_complete().

Skipping initiate_domain_create() means dcs->console_wait is
uninitialized and all 0s.

We have:
  domcreate_complete()
    libxl__xswait_stop()
      libxl__ev_xswatch_deregister().

The uninitialized slotnum 0 is considered valid (-1 is the invalid
sentinel), so the NULL pointer path to passed to xs_unwatch() which
segfaults.

libxl__ev_xswatch_deregister:watch w=0x12bc250 wpath=(null) token=0/0: deregister slotnum=0

Move dcs->console_xswait initialization into the callers of
initiate_domain_create, do_domain_create() and do_domain_soft_reset(),
so it is initialized along with the other dcs state.

Fixes: c57e6ebd8c ("(lib)xl: soft reset support")
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2022-04-01 17:01:57 +01:00
Jason Andryuk e7cfcdc671 libxl: constify libxl__stubdomain_is_linux
libxl__stubdomain_is_linux can take a const pointer, so make the change.

This isn't an issue in-tree, but was found with an OpenXT patch where it
was called with only const libxl_domain_build_info available.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2022-03-31 19:50:03 +01:00
Jason Andryuk e45ad0b1b0 xl: Fix global pci options
commit babde47a3f "introduce a 'passthrough' configuration option to
xl.cfg..." moved the pci list parsing ahead of the global pci option
parsing.  This broke the global pci configuration options since they
need to be set first so that looping over the pci devices assigns their
values.

Move the global pci options ahead of the pci list to restore their
function.

Fixes: babde47a3f ("introduce a 'passthrough' configuration option to xl.cfg...")
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2022-03-31 19:48:12 +01:00
Jason Andryuk 73ee2795aa libxl/PCI: Fix PV hotplug & stubdom coldplug
commit 0fdb48ffe7 "libxl: Make sure devices added by pci-attach are
reflected in the config" broken PCI hotplug (xl pci-attach) for PV
domains when it moved libxl__create_pci_backend() later in the function.

This also broke HVM + stubdom PCI passthrough coldplug.  For that, the
PCI devices are hotplugged to a running PV stubdom, and then the QEMU
QMP device_add commands are made to QEMU inside the stubdom.

A running PV domain calls libxl__wait_for_backend().  With the current
placement of libxl__create_pci_backend(), the path does not exist and
the call immediately fails:
libxl: error: libxl_device.c:1388:libxl__wait_for_backend: Backend /local/domain/0/backend/pci/43/0 does not exist
libxl: error: libxl_pci.c:1764:device_pci_add_done: Domain 42:libxl__device_pci_add failed for PCI device 0:2:0.0 (rc -3)
libxl: error: libxl_create.c:1857:domcreate_attach_devices: Domain 42:unable to add pci devices

The wait is only relevant when:
1) The domain is PV
2) The domain is running
3) The backend is already present

This is because:

1) xen-pcifront is only used for PV.  It does not load for HVM domains
   where QEMU is used.

2) If the domain is not running (starting), then the frontend state will
   be Initialising.  xen-pciback waits for the frontend to transition to
   at Initialised before attempting to connect.  So a wait for a
   non-running domain is not applicable as the backend will not
   transition to Connected.

3) For presence, num_devs is already used to determine if the backend
   needs to be created.  Re-use num_devs to determine if the backend
   wait is necessary.  The wait is necessary to avoid racing with
   another PCI attachment reconfiguring the front/back or changing to
   some other state like closing.  If we are creating the backend, then
   we don't have to worry about the state since it is being created.

Fixes: 0fdb48ffe7 ("libxl: Make sure devices added by pci-attach are
reflected in the config")

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
2022-01-13 14:33:16 +01:00
Jason Andryuk fdff034b07 vtpm: Correct timeout units and command duration
Add two patches:
vtpm-microsecond-duration.patch fixes the units for timeouts and command
durations.
vtpm-command-duration.patch increases the timeout linux uses to allow
commands to succeed.

Linux works around low timeouts, but not low durations.  The second
patch allows commands to complete that often timeout with the lower
command durations.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
2021-05-10 14:50:33 +01:00
Jason Andryuk 5974702bdd vtpmmgr: Check req_len before unpacking command
vtpm_handle_cmd doesn't ensure there is enough space before unpacking
the req buffer.  Add a minimum size check.  Called functions will have
to do their own checking if they need more data from the request.

The error case is tricky since abort_egress wants to rely with a
corresponding tag.  Just hardcode TPM_TAG_RQU_COMMAND since the vtpm is
sending in malformed commands in the first place.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
2021-05-10 14:50:33 +01:00
Jason Andryuk c8d8a1e2e6 vtpmmgr: Fix owner_auth & srk_auth parsing
Argument parsing only matches to before ':' and then the string with
leading ':' is passed to parse_auth_string which fails to parse.  Extend
the length to include the seperator in the match.

While here, switch the seperator to "=".  The man page documented "="
and the other tpm.* arguments already use "=".  Since it didn't work
before, we don't need to worry about backwards compatibility.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
2021-05-10 14:50:33 +01:00
Jason Andryuk 7f4276fad8 vtpmmgr: Remove bogus cast from TPM2_GetRandom
The UINT32 <-> UINT16 casting in TPM2_GetRandom is incorrect.  Use a
local UINT16 as needed for the TPM hardware command and assign the
result.

Suggested-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
2021-05-10 14:50:33 +01:00
Jason Andryuk 64d00dd126 vtpmmgr: Support GetRandom passthrough on TPM 2.0
GetRandom passthrough currently fails when using vtpmmgr with a hardware
TPM 2.0.
vtpmmgr (8): INFO[VTPM]: Passthrough: TPM_GetRandom
vtpm (12): vtpm_cmd.c:120: Error: TPM_GetRandom() failed with error code (30)

When running on TPM 2.0 hardware, vtpmmgr needs to convert the TPM 1.2
TPM_ORD_GetRandom into a TPM2 TPM_CC_GetRandom command.  Besides the
differing ordinal, the TPM 1.2 uses 32bit sizes for the request and
response (vs. 16bit for TPM2).

Place the random output directly into the tpmcmd->resp and build the
packet around it.  This avoids bouncing through an extra buffer, but the
header has to be written after grabbing the random bytes so we have the
number of bytes to include in the size.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed by: Daniel P. Smith <dpsmith@apertussolutions.com>
2021-05-10 14:50:27 +01:00
Jason Andryuk 1b4bfa0b8f vtpmmgr: Shutdown more gracefully
vtpmmgr uses the default, weak app_shutdown, which immediately calls the
shutdown hypercall.  This short circuits the vtpmmgr clean up logic.  We
need to perform the clean up to actually Flush our key out of the tpm.

Setting do_shutdown is one step in that direction, but vtpmmgr will most
likely be waiting in tpmback_req_any.  We need to call shutdown_tpmback
to cancel the wait inside tpmback and perform the shutdown.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibaut@ens-lyon.org>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
2021-05-10 14:49:01 +01:00
Jason Andryuk 746d20b112 vtpmmgr: Flush all transient keys
We're only flushing 2 transients, but there are 3 handles.  Use <= to also
flush the third handle since TRANSIENT_LAST is inclusive

The number of transient handles/keys is hardware dependent, so this
should query for the limit.  And assignment of handles is assumed to be
sequential from the minimum.  That may not be guaranteed, but seems okay
with my tpm2.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
2021-05-10 14:49:01 +01:00
Jason Andryuk ec9596dc53 vtpmmgr: Flush transient keys on shutdown
Remove our key so it isn't left in the TPM for someone to come along
after vtpmmgr shutsdown.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
2021-05-10 14:49:00 +01:00
Jason Andryuk 3f1b5080d6 vtpmmgr: Allow specifying srk_handle for TPM2
Bypass taking ownership of the TPM2 if an srk_handle is specified.

This srk_handle must be usable with Null auth for the time being.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
2021-05-10 14:48:13 +01:00
Jason Andryuk a7da84c457 vtpmmgr: Move vtpmmgr_shutdown
Reposition vtpmmgr_shutdown so it can call flush_tpm2 without a forward
declaration.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
2021-05-07 19:50:50 +01:00
Jason Andryuk 244fdf0027 stubom: newlib: Enable C99 formats for %z
vtpmmgr was changed to print size_t with the %z modifier, but newlib
isn't compiled with %z support.  So you get output like:

root seal: zu; sector of 13: zu
root: zu v=zu
itree: 36; sector of 112: zu
group: zu v=zu id=zu md=zu
group seal: zu; 5 in parent: zu; sector of 13: zu
vtpm: zu+zu; sector of 48: zu

Enable the C99 formats in newlib so vtpmmgr prints the numeric values.

Fixes: 9379af08cc ("stubdom: vtpmmgr: Correctly format size_t with %z when printing.")
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
2021-05-07 19:50:50 +01:00
Jason Andryuk 15a59d6ef3 vtpmmgr: Print error code to aid debugging
tpm_get_error_name returns "Unknown Error Code" when an error string
is not defined.  In that case, we should print the Error Code so it can
be looked up offline.  tpm_get_error_name returns a const string, so
just have the two callers always print the error code so it is always
available.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
2021-05-07 19:50:50 +01:00
Jason Andryuk 93b2558fae docs: Warn about incomplete vtpmmgr TPM 2.0 support
The vtpmmgr TPM 2.0 support is incomplete.  Add a warning about that to
the documentation so others don't have to work through discovering it is
broken.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
2021-05-07 19:50:50 +01:00
Jason Andryuk 0a5e0ce0fb libxl: Add suppress-vmdesc to QEMU machine
The device model state saved by QMP xen-save-devices-state doesn't
include the vmdesc json.  When restoring an HVM, xen-load-devices-state
always triggers "Expected vmdescription section, but got 0".  This is
not a problem when restore comes from a file.  However, when QEMU runs
in a linux stubdom and comes over a console, EOF is not received.  This
causes a delay restoring - though it does restore.

Setting suppress-vmdesc skips looking for the vmdesc during restore and
avoids the wait.

QEMU 5.2 enables suppress-vmdesc by default for xenfv, but this change
sets it manually for xenfv and xen_platform_pci=0 when -machine pc is
use.

QEMU commit 9850c6047b8b "migration: Allow to suppress vmdesc
submission" added suppress-vmdesc in QEMU 2.3.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
2020-11-06 16:20:36 +00:00
Jason Andryuk 4ddd6499d9 SUPPORT: Add linux device model stubdom to Toolstack
Add qemu-xen linux device model stubdomain to the Toolstack section as a
Tech Preview.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-10-23 17:14:08 +01:00
Jason Andryuk 1b810a9d5a libxl: only query VNC when enabled
QEMU without VNC support (configure --disable-vnc) will return an error
when VNC is queried over QMP since it does not recognize the QMP
command.  This will cause libxl to fail starting the domain even if VNC
is not enabled.  Therefore only query QEMU for VNC support when using
VNC, so a VNC-less QEMU will function in this configuration.

'goto out' jumps to the call to device_model_postconfig_done(), the
final callback after the chain of vnc queries.  This bypasses all the
QMP VNC queries.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Wei Liu <wl@xen.org>
2020-10-11 20:59:16 +00:00
Jason Andryuk 40b532fbdc vchan-socket-proxy: Handle closing shared input/output_fd
input_fd & output_fd may be the same FD.  In that case, mark both as -1
when closing one.  That avoids a dangling FD reference.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Wei Liu <wl@xen.org>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Release-acked-by: Paul Durrant <paul@xen.org>
2020-06-26 12:00:05 +00:00
Jason Andryuk d20c0f1a78 vchan-socket-proxy: Cleanup resources on exit
Close open FDs and close th vchan connection when exiting the program.

This addresses some Coverity findings about leaking file descriptors.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Wei Liu <wl@xen.org>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Release-acked-by: Paul Durrant <paul@xen.org>
2020-06-26 12:00:02 +00:00
Jason Andryuk 620225c485 vchan-socket-proxy: Set closed FDs to -1
These FDs are closed, so set them to -1 so they are no longer valid.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Wei Liu <wl@xen.org>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Release-acked-by: Paul Durrant <paul@xen.org>
2020-06-26 11:59:59 +00:00
Jason Andryuk bfb310ee68 vchan-socket-proxy: Switch data_loop() to take state
Switch data_loop to take a pointer to vchan_proxy_state.

No functional change.

This removes a dead store to input_fd identified by Coverity.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Wei Liu <wl@xen.org>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Release-acked-by: Paul Durrant <paul@xen.org>
2020-06-26 11:59:56 +00:00
Jason Andryuk ace450ebdf vchan-socket-proxy: Use a struct to store state
Use a struct to group the vchan ctrl and FDs.  This will facilite
tracking the state of open and closed FDs and ctrl in data_loop().

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Wei Liu <wl@xen.org>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Release-acked-by: Paul Durrant <paul@xen.org>
2020-06-26 11:59:53 +00:00
Jason Andryuk 2b1a218811 vchan-socket-proxy: Unify main return value
Introduce 'ret' for main's return value and remove direct returns.  This
is in preparation for a unified exit path with resource cleanup.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Wei Liu <wl@xen.org>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Release-acked-by: Paul Durrant <paul@xen.org>
2020-06-26 11:59:49 +00:00
Jason Andryuk 20b65c15a3 vchan-socket-proxy: Check xs_watch return value
Check the return value of xs_watch and error out on failure.

This was found by Citrix's Coverity.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Wei Liu <wl@xen.org>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Release-acked-by: Paul Durrant <paul@xen.org>
2020-06-26 11:59:46 +00:00
Jason Andryuk fbdf181fe3 vchan-socket-proxy: Move perror() into connect_socket
errno is reset by subsequent system & library calls, so it may be
inaccurate by the time connect_socket returns.  Call perror immediately
after failing system calls to print the proper message.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Wei Liu <wl@xen.org>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Release-acked-by: Paul Durrant <paul@xen.org>
2020-06-26 11:59:41 +00:00
Jason Andryuk 01b9a28e7d vchan-socket-proxy: Move perror() into listen_socket
The use of perror on the return from listen_socket can produce
misleading results like:
UNIX socket path "/tmp/aa....aa" too long (156 >= 108)
listen socket: Success

errno is reset by subsequent system & library calls, so it may be
inaccurate by the time listen_socket returns.  Call perror immediately
after failing system calls to print the proper message.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Wei Liu <wl@xen.org>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Release-acked-by: Paul Durrant <paul@xen.org>
2020-06-26 11:59:38 +00:00
Jason Andryuk 2c8ac47d4e vchan-socket-proxy: Ensure UNIX path NUL terminated
Check the socket path length to ensure sun_path is NUL terminated.

This was spotted by Citrix's Coverity.

Also use strcpy to avoid a warning "'__builtin_strncpy' specified bound
108 equals destination size [-Werror=stringop-truncation]" flagged by
gcc 10.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Wei Liu <wl@xen.org>
Reviewed-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Release-acked-by: Paul Durrant <paul@xen.org>
2020-06-26 11:58:30 +00:00
Jason Andryuk f0dca895a5 xl: Allow shutdown wait for domain death
`xl shutdown -w` waits for the first of either domain shutdown or death.
Shutdown is the halting of the guest operating system, and death is the
freeing of domain resources.

Allow specifying -w multiple times to wait for only domain death.  This
is useful in scripts so that all resources are free before the script
continues.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Paul Durrant <paul@xen.org>
2020-06-18 16:58:56 +01:00
Jason Andryuk d3db7e043c libacpi: widen TPM detection
The hardcoded tpm_signature is too restrictive to detect many TPMs.  For
instance, it doesn't accept a QEMU emulated TPM (VID 0x1014 DID 0x0001).
Make the TPM detection match that in rombios which accepts a wider
range.

With this change, the TPM's TCPA ACPI table is generated and the guest
OS can automatically load the tpm_tis driver.  It also allows seabios to
detect and use the TPM.  However, seabios skips some TPM initialization
when running under Xen, so it will not populate any PCRs unless modified
to run the initialization under Xen.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Paul Durrant <paul@xen.org>
2020-06-16 10:31:08 +02:00
Jason Andryuk 52a9022fe9 CHANGELOG: Add qemu-xen linux device model stubdomains
Add qemu-xen linux device model stubdomain.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Paul Durrant <paul@xen.org>
2020-06-02 12:03:56 +00:00
Jason Andryuk e235fa2794 libxl: Check stubdomain kernel & ramdisk presence
Just out of context is the following comment for libxl__domain_make:
/* fixme: this function can leak the stubdom if it fails */

When the stubdomain kernel or ramdisk is not present, the domid and
stubdomain name will indeed be leaked.  Avoid the leak by checking the
file presence and erroring out when absent.  It doesn't fix all cases,
but it avoids a big one when using a linux device model stubdomain.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-05-19 16:19:15 +01:00
Jason Andryuk b6af49af60 docs: Add device-model-domid to xenstore-paths
Document device-model-domid for when using a device model stubdomain.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-05-19 16:19:15 +01:00
Jason Andryuk 379ab27086 libxl: Refactor kill_device_model to libxl__kill_xs_path
Move kill_device_model to libxl__kill_xs_path so we have a helper to
kill a process from a pid stored in xenstore.  We'll be using it to kill
vchan-qmp-proxy.

libxl__kill_xs_path takes a "what" string for use in printing error
messages.  kill_device_model is retained in libxl_dm.c to provide the
string.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-05-19 16:19:15 +01:00
Jason Andryuk 6d721be59d libxl: Use libxl__xs_* in libxl__write_stub_dmargs
Re-work libxl__write_stub_dmargs to use libxl_xs_* functions in a loop.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-05-19 16:19:05 +01:00
Jason Andryuk b72682c602 scripts: Use stat to check lock claim
Replace the perl locking check with stat(1).  Stat is able to fstat
stdin (file descriptor 0) when passed '-' as an argument.  This is now
used to check $_lockfd.  stat(1) support for '-' was introduced to
coreutils in 2009.

After A releases its lock, script B will return from flock and execute
stat.  Since the lockfile has been removed by A, stat prints an error to
stderr and exits non-zero.  Redirect stderr to /dev/null to avoid
filling /var/log/xen/xen-hotplug.log with "No such file or directory"
messages.

Placing the stat call inside the "if" condition ensures we only check
the stat output when the command completed successfully.

This change removes the only runtime dependency of the xen toolstack on
perl.

Suggested-by: Ian Jackson <ian.jackson@citrix.com>
Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
2020-03-30 14:35:53 +01:00
Jason Andryuk 8ff7a8811a scripts: Replace tabs in locking.sh
Replace two stray tabs with spaces to make the file whitespace
consistent.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Wei Liu <wl@xen.org>
2020-03-13 10:18:21 +00:00
Jason Andryuk 32772fbb3c x86/shadow: use single (atomic) MOV for emulated writes
This is the corresponding change to the shadow code as made by
bf08a8a08a "x86/HVM: use single (atomic) MOV for aligned emulated
writes" to the non-shadow HVM code.

The bf08a8a08a commit message:
Using memcpy() may result in multiple individual byte accesses
(depending how memcpy() is implemented and how the resulting insns,
e.g. REP MOVSB, get carried out in hardware), which isn't what we
want/need for carrying out guest insns as correctly as possible. Fall
back to memcpy() only for accesses not 2, 4, or 8 bytes in size.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Acked-by: Tim Deegan <tim@xen.org>
2020-01-17 16:19:16 +01:00
Jason Andryuk deb8ba12e5 x86/tboot: avoid recursive fault in early boot panic with tboot
If panic is called before init_idle_domain on a tboot-launched system,
then Xen recursively faults in write_ptbase as seen below.

(XEN)    [<ffff82d080286690>] write_ptbase+0/0x10
(XEN)    [<ffff82d0802c4c3b>] tboot_shutdown+0x6b/0x260
(XEN)    [<ffff82d08029ddac>] machine_restart+0xac/0x2d0
(XEN)    [<ffff82d080286690>] write_ptbase+0/0x10
(XEN)    [<ffff82d0802446c1>] panic+0x111/0x120
(XEN)    [<ffff82d0802a51c1>] do_general_protection+0x171/0x1f0
(XEN)    [<ffff82d080287a82>] mm.c#virt_to_xen_l2e+0x12/0x1c0
(XEN)    [<ffff82d080354720>] x86_64/entry.S#handle_exception_saved+0x66/0xa4
(XEN)    [<ffff82d080286690>] write_ptbase+0/0x10
(XEN)    [<ffff82d0802c4c3b>] tboot_shutdown+0x6b/0x260
(XEN)    [<ffff82d08029ddac>] machine_restart+0xac/0x2d0
(XEN)    [<ffff82d0802446c1>] panic+0x111/0x120
(XEN)    [<ffff82d0803c11a0>] setup.c#bootstrap_map+0/0x11a
(XEN)    [<ffff82d0803b82a0>] flask_op.c#parse_flask_param+0/0xb0
(XEN)    [<ffff82d0803c11a0>] setup.c#bootstrap_map+0/0x11a
(XEN)    [<ffff82d0803b6f6c>] xsm_multiboot_init+0x7c/0xb0
(XEN)    [<ffff82d0803c34bb>] __start_xen+0x1d2b/0x2da0
(XEN)    [<ffff82d0802000f3>] __high_start+0x53/0x60

idle_vcpu[0] is still poisoned with INVALID_VCPU, so write_ptbase faults
dereferencing the pointer.  This fault calls panic and recurses through
the same code path.

If tboot_shutdown is called while idle_vcpu[0] == INVALID_VCPU, then we
are still operating with the initial page tables.  Therefore changing
page tables with write_ptbase is unnecessary.

An easy way to reproduce this is to use tboot to launch an XSM-enabled
Xen without an XSM policy.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2018-07-24 15:55:07 +02:00