Commit Graph
2952 Commits
Author SHA1 Message Date
Bertrand Marquis 8e0c2a2063 tool/libs/light: Fix libxenlight gcc warning
Fix gcc10 compilation warning about uninitialized variable by setting
it to 0.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
Acked-by: Wei Liu <wl@xen.org>
(cherry picked from commit 0241809bf8)
2020-11-09 17:58:18 +00:00
Marek Marczykowski-Górecki 34056b2b09 libxl: fix -Werror=stringop-truncation in libxl__prepare_sockaddr_un
In file included from /usr/include/string.h:495,
                 from libxl_internal.h:38,
                 from libxl_utils.c:20:
In function 'strncpy',
    inlined from 'libxl__prepare_sockaddr_un' at libxl_utils.c:1262:5:
/usr/include/bits/string_fortified.h:106:10: error: '__builtin_strncpy' specified bound 108 equals destination size [-Werror=stringop-truncation]
  106 |   return __builtin___strncpy_chk (__dest, __src, __len, __bos (__dest));
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit fff1b7f50e)
2020-11-09 17:58:13 +00:00
Marek Marczykowski-Górecki fd4cc0bc9b libxl: workaround gcc 10.2 maybe-uninitialized warning
It seems xlu_pci_parse_bdf has a state machine that is too complex for
gcc to understand. The build fails with:

    libxlu_pci.c: In function 'xlu_pci_parse_bdf':
    libxlu_pci.c:32:18: error: 'func' may be used uninitialized in this function [-Werror=maybe-uninitialized]
       32 |     pcidev->func = func;
          |     ~~~~~~~~~~~~~^~~~~~
    libxlu_pci.c:51:29: note: 'func' was declared here
       51 |     unsigned dom, bus, dev, func, vslot = 0;
          |                             ^~~~
    libxlu_pci.c:31:17: error: 'dev' may be used uninitialized in this function [-Werror=maybe-uninitialized]
       31 |     pcidev->dev = dev;
          |     ~~~~~~~~~~~~^~~~~
    libxlu_pci.c:51:24: note: 'dev' was declared here
       51 |     unsigned dom, bus, dev, func, vslot = 0;
          |                        ^~~
    libxlu_pci.c:30:17: error: 'bus' may be used uninitialized in this function [-Werror=maybe-uninitialized]
       30 |     pcidev->bus = bus;
          |     ~~~~~~~~~~~~^~~~~
    libxlu_pci.c:51:19: note: 'bus' was declared here
       51 |     unsigned dom, bus, dev, func, vslot = 0;
          |                   ^~~
    libxlu_pci.c:29:20: error: 'dom' may be used uninitialized in this function [-Werror=maybe-uninitialized]
       29 |     pcidev->domain = domain;
          |     ~~~~~~~~~~~~~~~^~~~~~~~
    libxlu_pci.c:51:14: note: 'dom' was declared here
       51 |     unsigned dom, bus, dev, func, vslot = 0;
          |              ^~~
    cc1: all warnings being treated as errors

Workaround it by setting the initial value to invalid value (0xffffffff)
and then assert on each value being set. This way we mute the gcc
warning, while still detecting bugs in the parse code.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit d25cc3ec93)
2020-11-09 17:58:13 +00:00
Andrew Cooper 06760c2bf3 tools/libxl: Fix memory leak in libxl_cpuid_set()
xc_cpuid_set() returns allocated memory via cpuid_res, which libxl needs to
free() seeing as it discards the results.

This is logically a backport of c/s b91825f628 "tools/libxc: Drop
config_transformed parameter from xc_cpuid_set()" but rewritten as one caller
of xc_cpuid_set() does use returned values.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
(cherry picked from commit c54de7d9df)
2020-06-15 15:46:17 +01:00
Andrew Cooper 9dc2842940 x86/spec-ctrl: CPUID/MSR definitions for Special Register Buffer Data Sampling
This is part of XSA-320 / CVE-2020-0543

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wl@xen.org>
(cherry picked from commit caab85ab58)
2020-06-09 17:45:37 +01:00
Anthony PERARD d1c9822b88 libxl: Fix comment about dcs.sdss
The field 'sdss' was named 'dmss' before, commit 3148bebbf0 did the
renamed but didn't update the comment.

Fixes: 3148bebbf0 ("libxl: rename a field in libxl__domain_create_state")
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
(cherry picked from commit 035c4d7716)
(cherry picked from commit 5351a0a170fc7f6290d7d3d8be302d53d2426a87)
2020-05-05 15:31:08 +01:00
Andrew Cooper a6366e0f88 x86: Enumeration for Control-flow Enforcement Technology
The CET spec has been published and guest kernels are starting to get support.
Introduce the CPUID and MSRs, and fully block the MSRs from guest use.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wl@xen.org>
master commit: 4803a67114
master date: 2020-04-21 16:52:03 +0100
2020-04-27 15:54:14 +02:00
George Dunlap 54e3018708 Rationalize max_grant_frames and max_maptrack_frames handling
Xen used to have single, system-wide limits for the number of grant
frames and maptrack frames a guest was allowed to create. Increasing
or decreasing this single limit on the Xen command-line would change
the limit for all guests on the system.

Later, per-domain limits for these values was created. The system-wide
limits became strict limits: domains could not be created with higher
limits, but could be created with lower limits. However, that change
also introduced a range of different "default" values into various
places in the toolstack:

- The python libxc bindings hard-coded these values to 32 and 1024,
  respectively
- The libxl default values are 32 and 1024 respectively.
- xl will use the libxl default for maptrack, but does its own default
  calculation for grant frames: either 32 or 64, based on the max
  possible mfn.

These defaults interact poorly with the hypervisor command-line limit:

- The hypervisor command-line limit cannot be used to raise the limit
  for all guests anymore, as the default in the toolstack will
  effectively override this.
- If you use the hypervisor command-line limit to *reduce* the limit,
  then the "default" values generated by the toolstack are too high,
  and all guest creations will fail.

In other words, the toolstack defaults require any change to be
effected by having the admin explicitly specify a new value in every
guest.

In order to address this, have grant_table_init treat negative values
for max_grant_frames and max_maptrack_frames as instructions to use the
system-wide default, and have all the above toolstacks default to passing
-1 unless a different value is explicitly configured.

This restores the old behavior in that changing the hypervisor command-line
option can change the behavior for all guests, while retaining the ability
to set per-guest values.  It also removes the bug that reducing the
system-wide max will cause all domains without explicit limits to fail.

NOTE: - The Ocaml bindings require the caller to always specify a value,
        and the code to start a xenstored stubdomain hard-codes these to 4
	and 128 respectively; this behavour will not be modified.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wl@xen.org>
Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
master commit: f2ae59bc4b
master date: 2019-11-29 21:43:49 +0000
2019-12-06 12:47:08 +01:00
Paul Durrant 694fa9cac8 passthrough: quarantine PCI devices
When a PCI device is assigned to an untrusted domain, it is possible for
that domain to program the device to DMA to an arbitrary address. The
IOMMU is used to protect the host from malicious DMA by making sure that
the device addresses can only target memory assigned to the guest. However,
when the guest domain is torn down the device is assigned back to dom0,
thus allowing any in-flight DMA to potentially target critical host data.

This patch introduces a 'quarantine' for PCI devices using dom_io. When
the toolstack makes a device assignable (by binding it to pciback), it
will now also assign it to DOMID_IO and the device will only be assigned
back to dom0 when the device is made unassignable again. Whilst device is
assignable it will only ever transfer between dom_io and guest domains.
dom_io is actually only used as a sentinel domain for quarantining purposes;
it is not configured with any IOMMU mappings. Assignment to dom_io simply
means that the device's initiator (requestor) identifier is not present in
the IOMMU's device table and thus any DMA transactions issued will be
terminated with a fault condition.

In addition, a fix to assignment handling is made for VT-d.  Failure
during the assignment step should not lead to a device still being
associated with its prior owner. Hand the device to DomIO temporarily,
until the assignment step has completed successfully.  Remove the PI
hooks from the source domain then earlier as well.

Failure of the recovery reassign_device_ownership() may not go silent:
There e.g. may still be left over RMRR mappings in the domain assignment
to which has failed, and hence we can't allow that domain to continue
executing.

NOTE: This patch also includes one printk() cleanup; the
      "XEN_DOMCTL_assign_device: " tag is dropped in iommu_do_pci_domctl(),
      since similar printk()-s elsewhere also don't log such a tag.

This is XSA-302.

Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
master commit: 319f9a0ba9
master date: 2019-10-31 16:20:05 +0100
2019-10-31 16:57:17 +01:00
Juergen Gross dcc0bf5dec libxl: fix pci device re-assigning after domain reboot
After a reboot of a guest only the first pci device configuration will
be retrieved from Xenstore resulting in loss of any further assigned
passed through pci devices.

The main reason is that all passed through pci devices reside under a
common root device "0" in Xenstore. So when the device list is rebuilt
from Xenstore after a reboot the sub-devices below that root device
need to be selected instead of using the root device number as a
selector.

Fix that by adding a new member to struct libxl_device_type which when
set is used to get the number of devices. Add such a member for pci to
get the correct number of pci devices instead of implying it from the
number of pci root devices (which will always be 1).

While at it fix the type of libxl__device_pci_from_xs_be() to match
the one of the .from_xenstore member of struct libxl_device_type. This
fixes a latent bug checking the return value of a function returning
void.

Signed-off-by: Juergen Gross <jgross@suse.com>
Tested-by: Chao Gao <chao.gao@intel.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

(cherry picked from commit c19434d928)
Backport-requested-by: Pasi Kärkkäinen <pasik@iki.fi>
Backport-requested-by: Roman Shaposhnik <roman@zededa.com>
Backport-requested-by: Rich Persaud <persaur@gmail.com>
2019-08-05 15:27:06 +01:00
Andrew Cooper 0a317c5162 x86/spec-ctrl: CPUID/MSR definitions for Microarchitectural Data Sampling
The MD_CLEAR feature can be automatically offered to guests.  No
infrastructure is needed in Xen to support the guest making use of it.

This is part of XSA-297, CVE-2018-12126, CVE-2018-12127, CVE-2018-12130, CVE-2019-11091.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit d4f6116c08)
2019-05-14 15:30:35 +01:00
Anthony PERARD ae29aa0f8f libxl: When restricted, start QEMU paused
libxl runs the command "cont" later during guest creation; i.e. it
is expecting that QEMU would not do any emulation.  Use the "-S"
command option to achieve this.

Unfortunately, when QEMU is started with "-S", it won't write QEMU's
readiness into xenstore. So only activate this option when we have a
QEMU startup notification via QMP available, i.e. when dm_restrict
is activated.

The -S option has the side-effect of suppressing the startup
notification via xenstore: libxl will only get the notification via
QMP.

It is important to rely only on QMP for notification when we have
QMP available, as (due to a qemu bug) not waiting for that QMP
notification may result in the QMP socket becoming blocked, so that
QEMU stops responding to new connections even if no existing ones
are active.

When the QEMU bug happens, the actions taken by both libxl and QEMU
are roughly as follows:
- libxl connects and handshakes with QEMU, then sends the
  cmd "query-status".
- QEMU prepares and maybe tries to send the response,
  while also writing "running" into xenstore.
- libxl sees via xenstore that QEMU is running and disconnects from the
  QMP socket before receiving the response from the cmd.
=> The QMP socket (monitor) is thereby blocked and will never reply
  to commands on new connections.

This is due to QEMU only responding to one command at a time, and
suspending its monitor (QMP) until the command has been processed and
sent. Disconnecting from the socket doesn't unsuspend the monitor. The
race described here is very likely to happen with QEMU 3.1.50 (during
3.2 development), but can be reproduced with QEMU 3.1.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-02-01 12:02:41 +00:00
Wei Liu f50dd67950 libxl: correctly dispose of dominfo list in libxl_name_to_domid
Tamas reported ssid_label was leaked. Use the designated function to
free dominfo list to fix the leakage.

Reported-by: Tamas K Lengyel <tamas@tklengyel.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Tested-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-01-29 14:57:58 +00:00
Wei Liu 1912f1220c libxl: fix error message for unsharing namespaces
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-01-18 14:22:36 +00:00
Ian Jackson 50923ade7a libxl: fix build (missing CLONE_NEWIPC) on astonishingly old systems
CLONE_NEWIPC was introduced in Linux 2.6.19, on the 29th of November
2006, which was 12 years, 1 month, and 14 days ago.

Nevertheless apparently some people are trying to build Xen on systems
whose kernel headers are that old.  Placate these people by providing
a fallback #define for CLONE_NEWIPC.

The actual binary value will of course remain constant, because of the
kernel API promise, so this is and will be correct on all platforms
where the CLONE_NEWIPC is supported.  (Even if for some reason we miss
the right #includes.)

Of course at runtime this value will not work on older kernels.  It
will be rejected as unknown by anything except some pre-2.6.18
kernels.  On those kernels we do not want to support dm_restrict, and
an attempt to use it will fail.  It is OK for the failure to be a
messy EINVAL syscall failure.  (The IPC namespace unshare is necessary
to avoid a suborned deprivileged qemu from causing trouble with shm,
sem, etc.)

On the very old kernels, the feature is totally out of scope.
(We are only interested, here, in making the build work, to avoid
blocking people who aren't using this feature.)

CC: Wei Liu <wei.liu2@citrix.com>
CC: Juergen Gross <jgross@suse.com>
CC: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-01-17 17:59:29 +00:00
Ian Jackson ba219192b1 Revert "libxl: fix build on rather old systems"
This reverts commit 1bce5f9baf.

This situation should be handled by disabling the dm restrict
feature, not silently falling back to lower protection.

Also this #ifdeffery is bad style.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-01-17 17:59:13 +00:00
Ian Jackson 93a62c544e libxl: get_reaper_lock_and_uid: Document fd handling
Coverity understandably complains that get_reaper_lock_and_uid leaks
the fd and hence open-file.  But this is intentional: the lock becomes
owned by the child process as a whole, which is entirely the property
of libxl.

(The coding style here in this subprocess is a bit anomalous but it's
probably not worth it to convert get_reaper_lock_and_uid to `goto out'
style and have it explicitly return the fd number.)

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Acked-by: George Dunlap <george.dunlap@citrix.com>
2019-01-11 17:34:26 +00:00
Jan Beulich 1bce5f9baf libxl: fix build on rather old systems
CLONE_NEWIPC has been introduced in Linux 2.6.19 only (and into glibc
at around that time as well). Cope with it being undefined as well as
with the underlying kernel not knowing of it.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Release-acked-by: Juergen Gross <jgross@suse.com>
2019-01-11 15:02:00 +00:00
Anthony PERARD 3729730e94 libxl: Add comments to libxl__json_*get* functions
This comments that libxl__json_object_get_* and libxl__json_*_get
functions accept the libxl__json_object parameter to be NULL.

libxl__json_object_to_json also works with NULL.

This also move libxl__json_object_alloc declaration closer to similar
functions, and closer to libxl__json_object_free.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-01-11 14:57:52 +00:00
Anthony PERARD 60351caf90 libxl_json: Remove libxl__json_object_append_to from header
It isn't possible to use libxl__json_object_append_to() outside of
libxl_json.c as there is no way to allocate a struct libxl__yajl_ctx.
So also remove libxl__yajl_ctx typedef from the internal header.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-01-11 14:57:52 +00:00
Anthony PERARD aa402adb0b libxl: Remove unused arg from libxl__sendmsg_fds
Now that `datalen' needs to be 1, we can remove it. Also change `data'
parameter to be a singe byte.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-01-11 14:57:52 +00:00
Anthony PERARD 2dc4e7b93b libxl: Re-implement domain_suspend_device_model using libxl__ev_qmp
The re-implementation is done because we want to be able to send the
file description that QEMU can use to save its state. When QEMU is
restricted, it would not be able to write to a path.

This replace both libxl__qmp_stop() and libxl__qmp_save().

qmp_qemu_check_version() was only used by libxl__qmp_save(), so it is
replace by a version using libxl__ev_qmp instead.

Coding style fixed in libxl__domain_suspend_device_model() for the
return value.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-01-11 14:57:52 +00:00
Anthony PERARD 6dda4a4d83 libxl: Change libxl__domain_suspend_device_model() to be async
This create an extra step for the two call sites of the function.

libxl__domain_suspend_device_model() in this patch gets an extra error
variable (there is ret and rc), but ret goes away in the next patch.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

---
libxl_domain_soft_reset() haven't been tested, as it doesn't appear to
possible to call the function from xl.
2019-01-11 14:57:52 +00:00
Anthony PERARD fdbe3394bb libxl_qmp: Store advertised QEMU version in libxl__ev_qmp
This will be used in a later patch.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-01-11 14:57:52 +00:00
Anthony PERARD f459c3ae33 libxl: QEMU startup sync based on QMP
This is only activated when dm_restrict=1, as explained in a previous
patch "libxl_dm: Pre-open QMP socket for QEMU"

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-01-11 14:57:52 +00:00
Anthony PERARD 785fb75168 libxl: Add dmss_init/dispose for libxl__dm_spawn_state
These two functions, dmss_init and dmss_dispose, need to be called to
initialise the private parts of a libxl__dm_spawn_state (dmss) as well
as dispose of them before giving back control to a caller.

There are 3 functions that can start using a dmss, the classic
libxl__spawn_local_dm, the one for stubdom libxl__spawn_stub_dm and
libxl__spawn_qdisk_backend. But there are only 2 exit path as
libxl__spawn_qdisk_backend is using libxl__spawn_local_dm functions.

These two new functions are empty but will be used shortly.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-01-11 14:57:52 +00:00
Anthony PERARD 77b595836b libxl_dm: Pre-open QMP socket for QEMU
This patch moves the creation of the QMP unix socket from QEMU to libxl.
But libxl doesn't rely on this yet.

When starting QEMU with dm_restrict=1, pre-open the QMP socket before
exec QEMU. That socket will be useful to find out if QEMU is ready, and
pre-opening it means that libxl can connect to it without waiting for
QEMU to create it.

The pre-opening is conditional, based on the use of dm_restrict
because it is using a new command line option of QEMU, and dm_restrict
support in QEMU is newer.

-chardev socket,fd=X is available with QEMU 2.12, since commit:
> char: allow passing pre-opened socket file descriptor at startup
> 0935700f8544033ebbd41e1f13cd528f8a58d24d

dm_restrict is available in QEMU 3.0.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-01-11 14:57:52 +00:00
Anthony PERARD e3f606ca1a libxl: Add init/dispose of for libxl__domain_build_state
These two new functions libxl__domain_build_state_{init,dispose} should
be called every time a new libxl__domain_build_state comes to existance.

There seems to be two of them, one with the domain creation machinery,
and one in the stub_dm_spawn.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-01-11 14:57:52 +00:00
Anthony PERARD 15c63d33ea libxl_exec: Add libxl__spawn_initiate_failure
This function can be used by user of libxl__spawn_* when they setup a
notification other than xenstore. The parent can already report success
via libxl__spawn_initiate_detach(), this new function can be used for
failure instead of waiting for the timeout.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-01-11 14:57:52 +00:00
Anthony PERARD a40e753fa1 libxl_qmp: Implementation of libxl__ev_qmp_*
This patch implement the API libxl__ev_qmp documented in the previous
patch, "libxl: Design of an async API to issue QMP commands to QEMU".

Since this API is to interact with QEMU via the QMP protocol, it also
implement a QMP client. The specification for the QEMU Machine Protocol
(QMP) can be found in the QEMU repository at:
https://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/interop/qmp-spec.txt

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
[ wei: fix build ]
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
2019-01-11 14:57:44 +00:00
Anthony PERARD db19ee2544 libxl: Design of an async API to issue QMP commands to QEMU
All the functions will be implemented in later patches.

This patch includes the API that libxl can use to send QMP commands to
QEMU.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
[ wei: fix build ]
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
2019-01-11 14:57:35 +00:00
Anthony PERARD 0778f1a499 libxl: Add wrapper around libxl__json_object_to_json JSON
That wrapper is going to be used to safely log a json_object, as
libxl__json_object_to_json return NULL on error. In the error case,
JSON() will return an invalid json string.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-01-11 14:57:35 +00:00
Anthony PERARD 535dface49 libxl_qmp: Change qmp_qemu_check_version to compare version
This patch makes the function simpler to read. It also add the ability
for a caller to tell if QEMU is newer or have the exact version.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-01-11 14:57:35 +00:00
Anthony PERARD 4a5a27c55e libxl_qmp: Separate QMP message generation from qmp_send_prepare
.. to be able to re-use qmp_prepare_cmd with libxl__ev_qmp.

This patch also add the QMP end of command '\r\n' into the generated
string as every caller will needs this.

There should be no functional change.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2019-01-11 14:57:35 +00:00
Anthony PERARD 394dd1d2d4 libxl: Enhance libxl__sendmsg_fds to deal with EINTR and EWOULDBLOCK
This patch change the behavior of libxl__sendmsg_fds to retry sendmsg on
EINTR error and return an error on short writes.

This patch allow a caller of libxl__sendmsg_fds to deal with EWOULDBLOCK
and short writes. The function now requires to send only 1 byte of data
so that when dealing with non-blocking fds a EWOULDBLOCK error would
mean that the fds haven't been sent yet. Current caller already send
only 1 byte.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
[ wei: fix build ]
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
2019-01-11 14:57:25 +00:00
Anthony PERARD 3fa1afc57e libxl_create: Re-order callbacks of initiate_domain_create
Callbacks should be in the order that there are going to be executed.
This patch fixes the initiate_domain_create callbacks, and also
reorders the callbacks prototypes. That way, it's easier to follow the
flow.

This patch:
- move libxl__colo_restore_setup_done after domcreate_bootloader_done.
- move domcreate_attach_devices after domcreate_devmodel_started.

No functional change.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
2019-01-02 14:16:52 +00:00
George Dunlap 82c62c8734 libxl/dm_depriv: Fix non-debug build
Apparently older versions of gcc, when building with debug=n, the
uninitialized variable logic gets confused.  Distros on which a
non-debug build will fail include:
- Centos 7
- Debian Jessie
- Ubuntu Trusty

It seems to be one particular path confusing the logic; so just set it
on that path to keep the compiler happy, while still catching other
potential paths where it might be unset.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
2018-12-22 08:59:48 +00:00
George Dunlap 9d357cbaf7 libxl/dm_depriv: Fix build
Build tests noticed a path where we weren't setting rc.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
2018-12-21 19:35:04 +00:00
George Dunlap f45e7d333f libxl: Introduce specific username to be used as a reaper
Untrusted device models must be killed by uid rather than by pid for
safety.  To do this reliably, we need another uid, not used for any
other purpose, from which to make the kill system call.

When using xen-qemuuser-range-base, we can repurpose
xen-qemuuser-range-base itself as a UID from which to kill other
devicemodel uids (since domain ID 0 should never have a device model
associated with it).

However, we'd like people to be able to use the device_model_user
feature without also defining xen-qemuuser-range-base (which requires
the ability to 'reserve' 32k+ user IDs).

To that end, introduce the xen-qemuuser-reaper id.  When killing by
UID, first look for and use that ID if available; then fall back to
xen-qemuuser-range-base.

Document the new call in docs/features/qemu-deprivilege.pandoc.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2018-12-21 18:42:00 +00:00
George Dunlap 87f9458e34 libxl: Kill QEMU with "reaper" ruid
Using kill(-1) to killing an untrusted dm process with the real uid
equal to the dm_uid isn't guaranteed to succeed: the process in
question may be able to kill the reaper process after the setresuid()
and before the kill().

Instead, set the real uid to the QEMU user for domain 0
(QEMU_USER_RANGE_BASE + 0).  The reaper process will still be able to
kill the dm process, but not vice versa.

This, in turn, requires locking to make sure that only one reaper
process is using that uid at a time; otherwise one reaper process may
kill the other reaper process.

Create a lockfile in RUNDIR/dm-reaper-lock, and grab the lock before
executing kill.

In the event that we can't get the lock for some reason, go ahead with
the kill using dm_uid for both real and effective UIDs.  This isn't
guaranteed to work, but it's no worse than not trying to kill the
process at all.

NB that this effectively requires admins using device_model_user to
also define xen_qemuuser_range_base; this will be addressed in
subsequent patches.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2018-12-21 18:41:59 +00:00
George Dunlap 0c653574d3 libxl: Kill QEMU by uid when possible
The privcmd fd that a dm_restrict'ed QEMU has gives it permission to
one specific domain ID.  This domain ID will probably eventually be
used again.  It is therefore necessary to make absolutely sure that a
rogue QEMU process cannot hang around after its domain has exited.

Killing QEMU by pid is insufficient in this situation, because QEMU
may be able to fork() to escape killing.  It is surprisingly tricky to
kill a process which can call fork() without races; the only reliable
way is to use kill(-1) to kill all processes with a given uid.

We can use this method only when we're sure that there's only one QEMU
instance per uid.  Add a dm_uid into the domain_build_state struct,
and set it in libxl__domain_get_device_model_uid() when it's safe to
kill by UID.  Store this in xenstore next to device-model-pid.

On domain destroy, check to see if device-model-uid is present in
xenstore.  If so, fork off a reaper process, setuid to that uid, and
do kill(-9) to kill all uids of that type.  Otherwise, carry on
destroying by pid.

While we're here, make libxl__destroy_device_model() consistently:
 1. Return an error when anything fails
 2. But continue to do as much clean-up as possible

NOTE that this is not yet completely safe: with ruid == dm_uid, the
device model may be able to kill(-9) the 'reaper' process before the
reaper process can kill it.  Further patches will address this.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2018-12-21 18:41:57 +00:00
George Dunlap 6a05c1a9b6 libxl: Make killing of device model asynchronous
Or at least, give it an asynchronous interface so that we can make it
actually asynchronous in subsequent patches.

Create state structures and callback function signatures.  Add the
state structure to libxl__destroy_domid_state.  Break
libxl__destroy_domid down into two functions.

No functional change intended.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2018-12-21 18:41:56 +00:00
George Dunlap a233781cd6 libxl: Move qmp cleanup into devicemodel destroy function
Removing the qmp connection is logically part of the device model
destruction; having the caller destroy it is a mild layering
violation.

Move libxl__qmp_cleanup() into libxl__destroy_device_model().  This
will make it easier when we make devicemodel destruction asynchronous.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2018-12-21 18:41:55 +00:00
George Dunlap 436bf83bf1 libxl: Do root checks once in libxl__domain_get_device_model_uid
At the moment, we check for equivalence to literal "root" before
deciding whether to add the `runas` command-line option to QEMU.  This
is unsatisfactory for several reasons.

First, just because the string doesn't match "root" doesn't mean the
final uid won't end up being zero; in particular, the range_base
calculations may end up producing "0:NNN", which would be root in any
case.

Secondly, it's almost certainly a configuration error if the resulting
uid ends up to be zero; rather than silently do what was specified but
probably not intended, throw an error.

To fix this, check for root once in
libxl__domain_get_device_model_uid.  If the result is root, return an
error; if appropriate, set `runas`.

After that, assume that the presence of state->dm_runas implies that a
`runas` argument should be constructed.

One side effect of this is to check whether device_model_user exists
before passing it to qemu, resulting in better error reporting.

While we're here:
- Refactor the function to use the "goto out" idiom
- Use 'rc' rather than 'ret', in line with CODING_STYLE

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2018-12-21 18:41:53 +00:00
George Dunlap 53bf85de47 dm_depriv: Describe expected usage of device_model_user parameter
A number of subsequent patches rely on as-yet undefined behavior for
what the `device_model_user` parameter does.  Rather than implement it
incorrectly (or randomly), or remove the feature, describe an expected
usage for the feature.  Further patches will make decisions based on
this expected usage.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2018-12-21 18:41:52 +00:00
George Dunlap 6572b66578 libxl: Clean up userlookup_helper_getpw* helper
Bring conventions more in line with libxl__xs_read_checked():
- If found, return 0 and set pointer to non-NULL
- If not found, return 0 and set pointer to NULL
- On error, return libxl-style error number.

Update documentation to match.

Use CODING_STYLE compliant `r` rather than `ret`.

On error, log the error code before returning instead of discarding
it.

Now that it only returns 0 or errno, update caller error checks to be
`if (ret)` rather than `if (ret < 0)`.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2018-12-21 18:41:51 +00:00
George Dunlap 28b5856b02 libxl: Get rid of support for QEMU_USER_BASE (xen-qemuuser-domidNN)
QEMU_USER_BASE allows a user to specify the UID to use when running
the devicemodel for a specific domain number.  Unfortunately, this is
not really practical: It requires nearly 32,000 entries in
/etc/passwd.  QEMU_USER_RANGE_BASE is much more practical.

Remove support for QEMU_USER_BASE.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2018-12-21 18:41:50 +00:00
George Dunlap 0fa6c9b344 libxl: Move dm user determination logic into a helper function
To reliably kill an untrusted devicemodel, we need to know not only
its pid, but its uid.  In preparation for this, move the userid
determination logic into a helper function.

Create a new field, `dm_runas`, in libxl__domain_build_state to store
the value during domain creation.

This change also removes unnecessary duplication of the argument
construction code.

While here, clean up some minor CODING_STYLE infractions (space
between * and variable name).

No functional change intended.

While here, delete some trailing whitespace.

Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2018-12-21 18:41:48 +00:00
Andrew Cooper 6b10a4c416 tools/docs: Remove PVRDTSCP support
PVRDTSCP is believed-unused, and its implementation has adverse consequences
on unrelated functionality in the hypervisor.  As a result, support is being
removed.

For more historical context, see
  c/s c17b36d5dc
  c/s 04656384a1

Modify libxl to provide a slightly more helpful error message if it encounters
PVRDTSCP being selected.  While adjusting TSC handling, make libxl check for
errors from the set_tsc hypercall.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2018-12-18 17:13:51 +00:00
Juergen Gross 0124380fdf tools/libxl: fix boot of HVM domain with Xenstore-stubdom
The Xenstore domid isn't set for HVM domains. This will result in
failure when booting a HVM domain on a system with Xenstore not running
in dom0.

Same applies for console domid, so set both.

This is broken since commit a2d9a6fa1f ("tools/libxenctrl: use new
xenforeignmemory API to seed grant table").

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
2018-12-07 12:19:17 +00:00