Commit Graph
28837 Commits
Author SHA1 Message Date
Zheng Li 1fb43d3e7c oxenstored: enable domain connection indexing based on eventchn port
Currently in xenstore connection database,  we use a hash table of
(domid -> connection) to store domain connections. This allows fast indexing
based on dom ids.

This patch adds another dimention of fast indexing that is based on eventchn
port number. This is useful when doing selective connection processing
based on the port numbers of incoming events.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Zheng Li <dev@zheng.li>
Reviewed-by: David Scott <dave.scott@citrix.com>
2017-04-05 15:26:37 +01:00
Zheng Li 0a8a4af826 oxenstored: use hash table to store socket connections
Currently we use list to store socket connections. This is fine for smaller
number of connections. But when we scale up, traveling through a list of
hundreds or thousands of connections just to find a single one of them is very
low efficient.

This patch replaces the list with a (Unix.file_descr -> Connection.t) hash table.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Zheng Li <dev@zheng.li>
Reviewed-by: David Scott <dave.scott@citrix.com>
2017-04-05 15:26:37 +01:00
Zheng Li edfddb3da8 oxenstored: catch the error when a connection is already deleted
The function process_fdset_with is called on the read set connections first.
During the process, it might destroy a connection and remove it from the
connections database if some errors occur. However, a reference to the same
connection might still exist in the write set, which is awaiting to be
processed next. In this case, a Not_found error will be raised and the process
is aborted.

This patch changes the logic to ignore connections just missing from the
connection database and continue the rest part of the work.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Zheng Li <dev@zheng.li>
Reviewed-by: David Scott <dave.scott@citrix.com>
2017-04-05 15:26:37 +01:00
Jerome Maloberti 53a6387e03 oxenstored: perform a 3-way merge of the quota after a transaction
At a beginning of a transaction, the quotas from the global store
are duplicated and modified by the transaction. If during the
transaction, an action associated to no transaction is concurrently
executed, the quotas of the global store are updated, and then the
updates are lost when the transaction merges.

We fix this problem by keeping another copy of the quota at the
beginning of the transaction, and performing a 3-way merge between
the quotas from the transaction and the "original" copy of the quota
onto the quota of the global store.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Jerome Maloberti <jerome.maloberti@citrix.com>
Signed-off-by: Euan Harris <euan.harris@citrix.com>
Acked-by: David Scott <dave.scott@citrix.com>
2017-04-05 15:26:37 +01:00
Vincent Bernardoff e6b2014af5 oxenstored: exempt dom0 from domU node quotas
If a domU has exhausted its quota we still want the toolstack in dom0 to
be able to create new nodes in places like
  /local/domain/%d/control/shutdown

Without this patch, a domU which has exhausted its quota can only be
powered off, which is not as good as being able to request a clean
shutdown.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Thomas Sanders <thomas.sanders@citrix.com>
Signed-off-by: Euan Harris <euan.harris@citrix.com>
Acked-by: David Scott <dave.scott@citrix.com>
2017-04-05 15:26:37 +01:00
Ian Jackson 3d00b3e8ac xenstored: Log when the write transaction rate limit bites
Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

plus:

xenstore: dont increment bool variable
Instead of incrementing a bool variable just set it to true.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2017-04-05 15:26:29 +01:00
Ian Jackson 3c7504e7d3 xenstored: apply a write transaction rate limit
This avoids a rogue client being about to stall another client (eg the
toolstack) indefinitely.

This is XSA-206.

Reported-by: Juergen Gross <jgross@suse.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
2017-04-05 15:26:21 +01:00
Jan Beulich 89bd1cf127 memory: properly check guest memory ranges in XENMEM_exchange handling
The use of guest_handle_okay() here (as introduced by the XSA-29 fix)
is insufficient here, guest_handle_subrange_okay() needs to be used
instead.

Note that the uses are okay in
- XENMEM_add_to_physmap_batch handling due to the size field being only
  16 bits wide,
- livepatch_list() due to the limit of 1024 enforced on the
  number-of-entries input (leaving aside the fact that this can be
  called by a privileged domain only anyway),
- compat mode handling due to counts there being limited to 32 bits,
- everywhere else due to guest arrays being accessed sequentially from
  index zero.

This is CVE-2017-7228 / XSA-212.

Reported-by: Jann Horn <jannh@google.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 938fd2586e
master date: 2017-04-04 14:47:46 +0200
2017-04-04 15:06:29 +02:00
Ian Jackson 9744389956 QEMU_TAG update 2017-03-21 18:45:09 +00:00
Ian Jackson 5e0448f4bb QEMU_TAG update 2017-02-22 16:46:48 +00:00
Oleksandr Tyshchenko b648113f8a IOMMU: always call teardown callback
There is a possible scenario when (d)->need_iommu remains unset
during guest domain execution. For example, when no devices
were assigned to it. Taking into account that teardown callback
is not called when (d)->need_iommu is unset we might have unreleased
resourses after destroying domain.

So, always call teardown callback to roll back actions
that were performed in init callback.

This is XSA-207.

Signed-off-by: Oleksandr Tyshchenko <olekstysh@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Jan Beulich <jbeulich@suse.com>
Tested-by: Julien Grall <julien.grall@arm.com>
2017-02-15 12:21:10 +00:00
Jan Beulich 394ddc2de6 x86: force EFLAGS.IF on when exiting to PV guests
Guest kernels modifying instructions in the process of being emulated
for another of their vCPU-s may effect EFLAGS.IF to be cleared upon
next exiting to guest context, by converting the being emulated
instruction to CLI (at the right point in time). Prevent any such bad
effects by always forcing EFLAGS.IF on. And to cover hypothetical other
similar issues, also force EFLAGS.{IOPL,NT,VM} to zero.

This is CVE-2016-10024 / XSA-202.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 0e47f92b07
master date: 2016-12-21 16:46:13 +0100
2016-12-21 17:48:43 +01:00
Andrew Cooper 5a343e41f5 x86/emul: Correct the handling of eflags with SYSCALL
A singlestep #DB is determined by the resulting eflags value from the
execution of SYSCALL, not the original eflags value.

By using the original eflags value, we negate the guest kernels attempt to
protect itself from a privilege escalation by masking TF.

Introduce a tf boolean and have the SYSCALL emulation recalculate it
after the instruction is complete.

This is XSA-204

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2016-12-19 15:46:47 +00:00
Jan Beulich 619db7dffb x86emul: CMPXCHG8B ignores operand size prefix
Otherwise besides mis-handling the instruction, the comparison failure
case would result in uninitialized stack data being handed back to the
guest in rDX:rAX (32 bits leaked for 32-bit guests, 96 bits for 64-bit
ones).

This is CVE-2016-9932 / XSA-200.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
2016-12-13 14:30:14 +01:00
Ian Jackson 149c34a6ea QEMU_TAG update 2016-12-07 16:54:47 +00:00
Ian Jackson 1c1bfc1cf2 QEMU_TAG update 2016-11-29 18:40:26 +00:00
Ian Jackson 6639a202f2 pygrub: Properly quote results, when returning them to the caller:
* When the caller wants sexpr output, use `repr()'
  This is what Xend expects.

  The returned S-expressions are now escaped and quoted by Python,
  generally using '...'.  Previously kernel and ramdisk were unquoted
  and args was quoted with "..." but without proper escaping.  This
  change may break toolstacks which do not properly dequote the
  returned S-expressions.

* When the caller wants "simple" output, crash if the delimiter is
  contained in the returned value.

  With --output-format=simple it does not seem like this could ever
  happen, because the bootloader config parsers all take line-based
  input from the various bootloader config files.

  With --output-format=simple0, this can happen if the bootloader
  config file contains nul bytes.

This is CVE-2016-9379 and CVE-2016-9380 / XSA-198.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Tested-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 27e14d346e
master date: 2016-11-22 13:52:09 +0100
2016-11-22 14:35:31 +01:00
Jan Beulich 14fa85aab3 x86emul: fix huge bit offset handling
We must never chop off the high 32 bits.

This is CVE-2016-9383 / XSA-195.

Reported-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 1c6c2d60d2
master date: 2016-11-22 13:49:06 +0100
2016-11-22 14:34:56 +01:00
Jan Beulich 1827d5252c x86/PV: writes of %fs and %gs base MSRs require canonical addresses
Commit c42494acb2 ("x86: fix FS/GS base handling when using the
fsgsbase feature") replaced the use of wrmsr_safe() on these paths
without recognizing that wr{f,g}sbase() use just wrmsrl() and that the
WR{F,G}SBASE instructions also raise #GP for non-canonical input.

Similarly arch_set_info_guest() needs to prevent non-canonical
addresses from getting stored into state later to be loaded by context
switch code. For consistency also check stack pointers and LDT base.
DR0..3, otoh, already get properly checked in set_debugreg() (albeit
we discard the error there).

The SHADOW_GS_BASE check isn't strictly necessary, but I think we
better avoid trying the WRMSR if we know it's going to fail.

This is CVE-2016-9385 / XSA-193.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: f3fa3abf3e
master date: 2016-11-22 13:46:28 +0100
2016-11-22 14:34:25 +01:00
Jan Beulich e8a46a2104 x86/HVM: don't load LDTR with VM86 mode attrs during task switch
Just like TR, LDTR is purely a protected mode facility and hence needs
to be loaded accordingly. Also move its loading to where it
architecurally belongs.

This is CVE-2016-9382 / XSA-192.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 93aa42b85a
master date: 2016-11-22 13:45:44 +0100
2016-11-22 14:33:54 +01:00
Andrew Cooper ed77368cc9 x86/hvm: Fix the handling of non-present segments
In 32bit, the data segments may be NULL to indicate that the segment is
ineligible for use.  In both 32bit and 64bit, the LDT selector may be NULL to
indicate that the entire LDT is ineligible for use.  However, nothing in Xen
actually checks for this condition when performing other segmentation
checks.  (Note however that limit and writeability checks are correctly
performed).

Neither Intel nor AMD specify the exact behaviour of loading a NULL segment.
Experimentally, AMD zeroes all attributes but leaves the base and limit
unmodified.  Intel zeroes the base, sets the limit to 0xfffffff and resets the
attributes to just .G and .D/B.

The use of the segment information in the VMCB/VMCS is equivalent to a native
pipeline interacting with the segment cache.  The present bit can therefore
have a subtly different meaning, and it is now cooked to uniformly indicate
whether the segment is usable or not.

GDTR and IDTR don't have access rights like the other segments, but for
consistency, they are treated as being present so no special casing is needed
elsewhere in the segmentation logic.

AMD hardware does not consider the present bit for %cs and %tr, and will
function as if they were present.  They are therefore unconditionally set to
present when reading information from the VMCB, to maintain the new meaning of
usability.

Intel hardware has a separate unusable bit in the VMCS segment attributes.
This bit is inverted and stored in the present field, so the hvm code can work
with architecturally-common state.

This is CVE-2016-9386 / XSA-191.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
master commit: 04beafa8e6
master date: 2016-11-22 13:44:50 +0100
2016-11-22 14:32:39 +01:00
Jan Beulich 488b7d28ab x86emul: honor guest CR0.TS and CR0.EM
We must not emulate any instructions accessing respective registers
when either of these flags is set in the guest view of the register, or
else we may do so on data not belonging to the guest's current task.

Being architecturally required behavior, the logic gets placed in the
instruction emulator instead of hvmemul_get_fpu(). It should be noted,
though, that hvmemul_get_fpu() being the only current handler for the
get_fpu() callback, we don't have an active problem with CR4: Both
CR4.OSFXSR and CR4.OSXSAVE get handled as necessary by that function.

This is XSA-190.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2016-10-04 14:06:16 +01:00
Jan Beulich dfddbf35d9 evtchn-fifo: prevent use after free
evtchn_fifo_init_control() calls evtchn_fifo_destroy() on an error
path, leading to cleanup_event_array() which frees d->evtchn_fifo
without also clearing the pointer. Otoh the bulk of
evtchn_fifo_init_control() is dependent on d->evtchn_fifo being NULL.

This is XSA-188 / CVE-2016-7154.

Reported-by: Mikhail V Gorobets <mikhail.v.gorobets@intel.com>
Suggested-by: Mikhail V Gorobets <mikhail.v.gorobets@intel.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2016-09-08 14:32:51 +02:00
Andrew Cooper 2f3e08d37b x86/shadow: Avoid overflowing sh_ctxt->seg_reg[]
hvm_get_seg_reg() does not perform a range check on its input segment, calls
hvm_get_segment_register() and writes straight into sh_ctxt->seg_reg[].

x86_seg_none is outside the bounds of sh_ctxt->seg_reg[], and will hit a BUG()
in {vmx,svm}_get_segment_register().

HVM guests running with shadow paging can end up performing a virtual to
linear translation with x86_seg_none.  This is used for addresses which are
already linear.  However, none of this is a legitimate pagetable update, so
fail the emulation in such a case.

This is XSA-187 / CVE-2016-7094.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Tim Deegan <tim@xen.org>
master commit: a9f3b3bad1
master date: 2016-09-08 14:16:26 +0200
2016-09-08 14:32:16 +02:00
Jan Beulich fbe0fb4b52 x86/32on64: don't allow recursive page tables from L3
L3 entries are special in PAE mode, and hence can't reasonably be used
for setting up recursive (and hence linear) page table mappings. Since
abuse is possible when the guest in fact gets run on 4-level page
tables, this needs to be excluded explicitly.

This is XSA-185 / CVE-2016-7092.

Reported-by: Jérémie Boutoille <jboutoille@ext.quarkslab.com>
Reported-by: "栾尚聪(好风)" <shangcong.lsc@alibaba-inc.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: c844d637d9
master date: 2016-09-08 14:14:53 +0200
2016-09-08 14:30:11 +02:00
Andrew Cooper 0fe7d69617 x86/pv: Remove unsafe bits from the mod_l?_entry() fastpath
All changes in writeability and cacheability must go through full
re-validation.

Rework the logic as a whitelist, to make it clearer to follow.

This is XSA-182

Reported-by: Jérémie Boutoille <jboutoille@ext.quarkslab.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Tim Deegan <tim@xen.org>
2016-07-26 14:11:29 +01:00
Ian Jackson 36a5a87850 QEMU_TAG update 2016-06-10 13:11:08 +01:00
Wei Liu 27f0143ef5 libxl: set XEN_QEMU_CONSOLE_LIMIT for QEMU
XSA-180 provides a patch to QEMU to bodge QEMU logging issue. We
explicitly set the limit in libxl for 4.7.

Introduce a function for setting the environment variable and call it in
the right places.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
(cherry picked from commit b0d409d9c4)

(cherry picked from commit fe82a96a65)
Conflicts:
	tools/libxl/libxl_dm.c

This version of libxl does not pass a dm_envs to the
*build_device_model_args* functions.  Instead, call
libxl__set_qemu_env_for_xsa_180 in libxl__spawn_local_dm.

The other call ultimate site of *build_device_model_args* (ie of
libxl__build_device_model_args) is in libxl__spawn_stub_dm, where we
don't need to set the env var.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 44a703dfbe)
2016-06-10 12:43:43 +01:00
Ian Jackson ec5925c72c libxl: Fix NULL pointer due to XSA-178 fix wrong XS nodename
In "libxl: Do not trust backend for disk eject vdev" (c69871a2fb on
xen.git#staging) we changed libxl_evenable_disk_eject to read the
device vdev out of xenstore from the /libxl path, rather than the
backend path, and to read it during setup rather than on each event.

However, the patch has a mistake:
    -        GCSPRINTF("%s/dev", backend), NULL);
    +        GCSPRINTF("%s/vdev", libxl_path), &configured_vdev);
                           ^
Spot the extra "v".  This causes configured_vdev always to be NULL.
configured_vdev is passed to [libxl__]strdup.

In Xen 4.6 and later libxl__strdup is used and tolerates NULL.
evg->vdev is set to NULL.  This propagates to the `vdev' field in the
generated event.  This may or may not cause further trouble, depending
on the calling application.  In our osstest test cases it does not
cause any trouble, so the bug goes undetected.

In Xen 4.5 and earlier, the strdup does not tolerate NULL, and libxl
crashes immediately.  This has been detected by osstest as a
regression in Xen 4.5.

IMO this patch should be applied immediately to
  xen.git#staging-4.5 (to check that it fixes the osstest regression)
  xen.git#staging     (to check that it does not break master

Subject to passes, it should then be propagated to all supported
stable trees and also be mentioned in an update to XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
CC: security@xenproject.org
CC: Jan Beulich <jbeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
(cherry picked from commit 62b4d4769c)
(cherry picked from commit 8b7a356409)
(cherry picked from commit 6d272988fb)
2016-06-08 16:15:55 +01:00
Ian Jackson aea2669313 libxl: Document ~/serial/ correctly
xenstore-paths.markdown talked about ~/device/serial/, but that's not
used.

(It is very wrong for this value, which contains a driver domain
filesystem path, to be in the guest's area of xenstore.  However, it
is only ever created by libxl and ready by xenconsoled.  When it is
created, it inherits the read-only permissions of /local/domain/DOMID.
So there is no security bug.)

This is a followup to XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 16:24:41 +01:00
Ian Jackson 8ce712fc84 libxl: Cleanup: use libxl__backendpath_parse_domid in libxl__device_disk_from_xs_be
Rather than an open-coded sscanf.  No functional change with correct
input.

This is a followup to XSA-175 and XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 16:24:41 +01:00
Ian Jackson 45c12108db libxl: Cleanup: Have libxl__alloc_vdev use /libxl
When allocating a vdev for a new disk, look in /libxl/device, rather
than the frontends directory in xenstore.

This is more in line with the other parts of libxl, which ought not to
trust frontends.  In this case, though, there is no security bug prior
to this patch because the frontend is the toolstack domain itself.

If libxl__alloc_vdev were ever changed to take a frontend domain
argument, this patch will fix a latent security bug.

This is a followup to XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 16:24:41 +01:00
Ian Jackson a0b99aba04 libxl: Do not trust backend for nic in list
libxl_device_nic_list should use the /libxl path to search for
devices, and for obtaining the device information.

The "type" parameter was always "vif".  Abolish it.  (In any case,
paths in /libxl/device are named after the frontend type which is
constant, not the backend type which might in future vary.)

Abolish a redundant store to pnic->backend_domid.  Before this commit,
that store was not needed because libxl_device_nic_init (called by
libxl__device_nic_from_xenstore) would zero it.  Now it overwrites the
correct backend domid with zero; so remove it.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 16:24:41 +01:00
Ian Jackson 9b2061daa5 libxl: Do not trust backend for nic in devid_to_device
libxl_devid_to_device_nic should read the information it needs from
the /libxl/device path, not the backend.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 16:24:41 +01:00
Ian Jackson 35fe0d6663 libxl: Do not trust backend in nic getinfo
This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 16:24:41 +01:00
Ian Jackson 7e42cb64bc libxl: Have READ_LIBXLDEV use libxl_path rather than be_path
Fix the just-introduced bug in this macro: now it reads the
trustworthy libxl_path.  Change the variable name in the two functions
(nic and channel) which use it.

Shuffling the bump in the carpet along, we now introduce three new
bugs: the three call sites pass a backend path where a frontend path
is expected.

No functional change.

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 16:24:41 +01:00
Ian Jackson c1f9a268b8 libxl: Rename READ_BACKEND to READ_LIBXLDEV
We are going to want to change all the functions that use READ_BACKEND
to get untrustworthy information from the backend, to use trustworthy
information from /libxl.

This will involve replacing READ_BACKEND, which reads from be_path,
with a similar macro READ_LIBXLDEV, which reads from libxl_path.

The macro name change generates a lot of clutter in the diff.  So we
break it out into this separate patch.  Here, we rename the macro, but
the implementation does not really match the new name.

So, another way to look at this, is that we have transformed the bug:
 * All of the backends use READ_BACKEND, which is unsafe
into the new bug:
 * READ_LIBXLDEV actually reads be_path, which is unsafe.

There is no functional change as yet.

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 16:24:41 +01:00
Ian Jackson 0eb5ef2986 libxl: Rename libxl__device_nic_from_xs_be to _from_xenstore
We are going to change these functions to expect, and be passed, a
/libxl path.  So it is wrong that they are called _from_xs_be.

Neither function reads anything which isn't found in both places, so
we can and will change the call sites later.

The only remaining function in libxl called *_from_xs_be relates to
PCI devices, for which the backend domain is hardcoded to 0 throughout
the libxl_pci.c.

No functional change.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 16:24:34 +01:00
Ian Jackson 98d7429a69 libxl: Do not trust backend for cdrom insert
Use the /libxl path where appropriate.  Rename `path' variable to
`be_path' to make sure we caught all the occurrences.

Specifically, when checking that the device still exists, check the
`frontend' value in /libxl, rather than anything in the backend
directory.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 16:19:02 +01:00
Ian Jackson 2dbe363169 libxl: Do not trust backend for disk in getinfo
Do not read the frontend path out of the backend.  We have it in our
hand.  Likewise the guest (frontend) domid was one of our parameters (!)

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 16:14:00 +01:00
Ian Jackson 5f07492725 libxl: Do not trust backend for disk; fix driver domain disks list
Rework libxl__device_disk_from_xs_be (which takes a backend path) into
to libxl__device_disk_from_xenstore (which takes a libxl path).

libxl__device_disk_from_xenstore now finds the backend path itself,
although it doesn't use it any more for most of its functions.  We
rename the variable from be_path to backend_path to make sure we
didn't miss any cases.

All the data collection is now done by reading from the copy in
/libxl.

libxl_device_disk_list and its helper libxl__append_disk_list (which
used to be libxl__append_disk_list_of_type) need extensive rework,
because they now need to specify the /libxl path rather than the
backend path.

To do that they enumerate disks by looking in the appropriate area in
/libxl.  Previously they scanned various of the backend directories in
dom0 (which was broken for driver domains).  It is no longer necessary
to enumerate the various disk backends, because they all use the same
paths in /devices.  libxl__device_disk_from_xenstore will parse the
type out of the backend path, for itself.  (Indeed, it did so before -
the now-gone type parameter to libxl__append_disk_list_of_type wasn't
used other than to construct the directory to list.)

Finally, remove a redundant store to pdisk->backend_domid in
libxl__append_disk_list[_of_type].  Even before this commit, that
store was not needed because libxl_device_disk_init (called by
libxl__device_disk_from_xenstore) would zero it.  Now it overwrites
the correct backend domid with zero; so remove it.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
---
v2: Also fix up COLO reads, following rebase
2016-06-06 16:14:00 +01:00
Ian Jackson 35343222ae libxl: Do not trust backend for disk eject vdev
For disk eject, use configured vdev from /libxl, not backend.

The backend directory is writeable by driver domains.  This means that
a malicious driver domain could cause libxl to see a wrong vdev,
confusing the user or the toolstack.

Use the vdev from the /libxl space, rather than the backend.

For convenience, we read the vdev from the /libxl space into the evg
during setup and copy it on each event, rather than reading it afresh
each time (which would in any case involve generating or saving a copy
of the relevant /libxl path).

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 16:14:00 +01:00
Ian Jackson 64282177f4 libxl: cdrom eject and insert: write to /libxl
Copy the new type and params values to /libxl, so that the information
in /libxl is kept up to date.

This is needed so that we can return this trustworthy information,
rather than trusting the backend-writeable parts of xenstore.

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 16:14:00 +01:00
Ian Jackson 17d70465f9 libxl: Do not trust backend for vtpm in getinfo (uuid)
Use uuid from /libxl, rather than from backend.  I think the backend
is not supposed to change the uuid, since it seems to be set by libxl
during setup.

If in fact the backend is supposed to be able to change the uuid, this
patch needs to be dropped and replaced by a patch which makes the vtpm
uuid lookup tolerate bad or missing data.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 16:01:14 +01:00
Ian Jackson 7bd50f1bd6 libxl: Do not trust backend for vtpm in getinfo (except uuid)
* Do not check the backend for existence.  We have already read the
  /libxl path so know that the vtpm exists (or is supposed to); if the
  backend doesn't exist then that must be the backend's doing.
* Get the frontend path from the /libxl directory.
* The frontend domid is the guest domid, and does not need to be read
  from xenstore (!)

We still attempt to read the uuid from the backend.  This will be
fixed in the next patch.

This is part of XSA-178.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 16:01:13 +01:00
Ian Jackson 6f76ac2cd3 libxl: Make copy of every xs backend in /libxl in _generic_add
We want to stop libxl trustingly reading information from the backend
directory (since this is, of course, writeable by the backend, which
might be a semi-trusted driver domain).

In principle it is wrong in current libxl for anything to try to
divine virtual device configuration from xenstore: the JSON domain
config ought to supply that, and xenstore should only tell us which
devices actually exist.

However:

Firstly, there are several existing places where configuration
information is retrieved from xenstore rather than JSON.  We do not
want to reen gineer this in a security patch.

Secondly, we want to make a security patch which can be backported to
versions of libxl without the JSON configuration machinery.

So we take the expedient approach of keeping a copy of the
configuration somewhere we trust, namely /libxl.  This is obviously
fairly low-risk, although it does write significantly more keys in
xenstore.

In this patch we make this change in libxl__device_generic_add.  This
is responsible for actually writing the vast majority of device
information to xenstore.  There are a few loose ends which will be
dealt with in a moment.

Likewise, changes to readers to use the new location will appear in
further patches.

This is part of XSA-178.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 15:59:51 +01:00
Ian Jackson 5519488064 libxl: Do not trust frontend for nic in getinfo
libxl_device_nic_getinfo needs to examine devices without trusting
frontend-controlled data.  So:

* Use /libxl to find the backend path.
* Parse the backend path to find the backend domid, rather than
  reading it from the frontend.

This is part of XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 15:54:27 +01:00
Ian Jackson 04e831a92b libxl: Do not trust frontend for nic in libxl_devid_to_device_nic
Find the backend by reading the pointer in /libxl rather than in the
guest's frontend area.

This is part of XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 15:54:27 +01:00
Ian Jackson 9b7d6d20bc libxl: Do not trust frontend for vtpm in getinfo
libxl_device_vtpm_getinfo needs to examine devices without trusting
frontend-controlled data.  So:

* Use /libxl to find the backend path.
* Parse the backend path to find the backend domid, rather than
  reading it from the frontend.

This is part of XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 15:54:27 +01:00
Ian Jackson 76a62af541 libxl: Do not trust frontend for vtpm list
libxl_device_vtpm_list needs to enumerate and identify devices without
trusting frontend-controlled data.  So

* Use the /libxl path to enumerate vtpms.
* Use the /libxl path to find the corresponding backends.
* Parse the backend path to find the backend domid.

This is part of XSA-175.

Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
2016-06-06 15:54:27 +01:00