Yang Zhang [Thu, 8 Aug 2013 08:36:22 +0000 (10:36 +0200)]
Nested VMX: Flush TLBs and Caches if paging mode changed
According to SDM, if paging mode is changed, then whole TLBs and caches will
be flushed. This is missed in nested handle logic. Also this fixed the issue
that 64 bits windows cannot boot up on top of L1 kvm.
Jan Beulich [Thu, 8 Aug 2013 08:34:31 +0000 (10:34 +0200)]
x86: refine FPU selector handling code for XSAVEOPT
Some extra tweaks are necessary to deal with the situation of XSAVEOPT
not writing the FPU portion of the save image (due to it detecting that
the register state did not get modified since the last XRSTOR).
Signed-off-by: Jan Beulich <jbeulich@suse.com> Tested-by: Ben Guthro <ben.guthro@gmail.com> Acked-by: Keir Fraser <keir@xen.org>
master commit: c58d9f2f4844c2ce8859a8d0f26a54cd058eb51f
master date: 2013-08-05 18:42:37 +0200
Andrew Cooper [Thu, 8 Aug 2013 08:33:42 +0000 (10:33 +0200)]
x86/time: Update wallclock in shared info when altering domain time offset
domain_set_time_offset() udpates d->time_offset_seconds, but does not correct
the wallclock in the shared info, meaning that it is incorrect until the next
XENPF_settime hypercall from dom0 which resynchronises the wallclock for all
domains.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Keir Fraser <keir@xen.org>
master commit: 915a59f25c5eddd86bc2cae6389d0ed2ab87e69e
master date: 2013-07-18 09:16:15 +0200
Andrew Cooper [Thu, 8 Aug 2013 08:32:56 +0000 (10:32 +0200)]
x86/cpuidle: Change logging for unknown APIC IDs
Dom0 uses this hypercall to pass ACPI information to Xen. It is not very
uncommon for more cpus to be listed in the ACPI tables than are present on the
system, particularly on systems with a common BIOS for a 2 and 4 socket server
varients.
As Dom0 does not control the number of entries in the ACPI tables, and is
required to pass everything it finds to Xen, change the logging.
There is now an single unconditional warning for the first unknown ID, and
further warnings if "cpuinfo" is requested by the user on the command line.
Jan Beulich [Thu, 8 Aug 2013 08:32:13 +0000 (10:32 +0200)]
adjust x86 EFI build
While the rule to generate .init.o files from .o ones already correctly
included $(extra-y), the setting of the necessary compiler flag didn't
have the same. With some yet to be posted patch this resulted in build
breakage because of the compiler deciding not to inline a few functions
(which then results in .text not being empty as required for these
object files).
Andrew Cooper [Thu, 8 Aug 2013 08:30:07 +0000 (10:30 +0200)]
x86/mm: Ensure useful progress in alloc_l2_table()
While debugging the issue which turned out to be XSA-58, a printk in this loop
showed that it was quite easy to never make useful progress, because of
consistently failing the preemption check.
One single l2 entry is a reasonable amount of work to do, even if an action is
pending, and also assures forwards progress across repeat continuations.
Tweak the continuation criteria to fail on the first iteration of the loop.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Keir Fraser <keir@xen.org>
master commit: d3a55d7d9bb518efe08143d050deff9f4ee80ec1
master date: 2013-07-04 10:33:18 +0200
Daniel Kiper [Tue, 7 May 2013 11:51:38 +0000 (13:51 +0200)]
tools/debugger/kdd: Remove dependencies files during make clean
Remove dependencies files during make clean.
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 38bdfb9197b93262248ff489eed336d80db52b54)
Daniel Kiper [Tue, 7 May 2013 11:51:39 +0000 (13:51 +0200)]
tools/xenmon: Fix typo in Makefile
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit ea5e515ba19c423e15ca33023cd3c9d2c9aa807f)
Daniel Kiper [Tue, 7 May 2013 11:51:40 +0000 (13:51 +0200)]
tools/xenstat/libxenstat: Remove src/libxenstat.a file during make clean
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 03b90b005939416463c79a45d91729e8a00742fa)
Daniel Kiper [Tue, 7 May 2013 11:51:43 +0000 (13:51 +0200)]
stubdom: Do not create dangling links
There is not architecture dependent files in libxc
hence do not create dangling links.
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> Acked-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
(cherry picked from commit c2eea87c43d1617b2c15c57fce9a64a436679fca)
Daniel Kiper [Tue, 7 May 2013 11:51:45 +0000 (13:51 +0200)]
docs: Remove tmp files during make clean
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit e17295d050110bdbbe0ef19c6e977c8fef7557db)
Daniel Kiper [Fri, 10 May 2013 15:33:54 +0000 (17:33 +0200)]
tools/libfsimage: Fix clean and distclean make targets
If there is a single colon for a given target and the target
is redefined in another place (e.g. in included file) then
make executes only new target and displays following warning:
Makefile:35: warning: overriding commands for target `clean'
tools/libfsimage/common/../../../tools/libfsimage/Rules.mk:25:
warning: ignoring old commands for target `clean'
To cope with that issue define all required targets as double-colon
rules. Additionally, remove some redundant stuff.
Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
(cherry picked from commit 667d8a84b244d02e9c6a2d02d6a02fc90c2efb4e)
pygrub/GrubConf: fix boot problem for fedora 19 grub.cfg (2nd attempt)
Booting a fedora 19 domU failed because a it could not properly
parse the grub.cfg file. This was cased by
set default="${next_entry}"
This statement actually is within an 'if' statement, so maybe it would
be better to skip code within if/fi blocks...
But this patch seems to work fine.
Signed-off-by: Marcel Mol <marcel@mesa.nl> Acked-by: Ian Campbell <ian.campbell@citix.com> Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
(cherry picked from commit d513814db6af2b298b8776d7ffc5fb1261e176f4)
Ian Murray [Thu, 4 Jul 2013 23:42:50 +0000 (23:42 +0000)]
Fix issue with 'xl list -l' showing domids as -1 when using SXP
During investigation of other issues, it came to light that in at least
4.2.2, "xl list -l" displays domain ids as -1 when using SXP, irrespective
of actual value. Ian C identified that this issue was likely fixed in the
upcoming 4.3 release but the commit responsible for the fix
(a73a7a0c647a9a5e30d8bc473c0a1e8648817183) was not likely a candidate for
backporting in its entirety.
Therefore, this patch is just an isolation of the hunk to fix the above issue.
Original Commit Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Backport Created-by: Ian Murray <murrayie@yahoo.co.uk>
The IOMMU interrupt handling in bottom half must clear the PPR log interrupt
and event log interrupt bits to re-enable the interrupt. This is done by
writing 1 to the memory mapped register to clear the bit. Due to hardware bug,
if the driver tries to clear this bit while the IOMMU hardware also setting
this bit, the conflict will result with the bit being set. If the interrupt
handling code does not make sure to clear this bit, subsequent changes in the
event/PPR logs will no longer generating interrupts, and would result if
buffer overflow. After clearing the bits, the driver must read back
the register to verify.
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Adjust to apply on top of heavily modified patch 1. Adjust flow to get away
with a single readl() in each instance of the status register checks.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Tim Deegan <tim@xen.org> Acked-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
master commit: 9eabb0735400e2b6059dfa3f0b47a426f61f570a
master date: 2013-07-02 08:50:41 +0200
iommu/amd: Fix logic for clearing the IOMMU interrupt bits
The IOMMU interrupt bits in the IOMMU status registers are
"read-only, and write-1-to-clear (RW1C). Therefore, the existing
logic which reads the register, set the bit, and then writing back
the values could accidentally clear certain bits if it has been set.
The correct logic would just be writing only the value which only
set the interrupt bits, and leave the rest to zeros.
This patch also, clean up #define masks as Jan has suggested.
Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
With iommu_interrupt_handler() properly having got switched its readl()
from status to control register, the subsequent writel() needed to be
switched too (and the RW1C comment there was bogus).
Some of the cleanup went too far - undone.
Further, with iommu_interrupt_handler() now actually disabling the
interrupt sources, they also need to get re-enabled by the tasklet once
it finished processing the respective log. This also implies re-running
the tasklet so that log entries added between reading the log and re-
enabling the interrupt will get handled in a timely manner.
Finally, guest write emulation to the status register needs to be done
with the RW1C (and RO for all other bits) semantics in mind too.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Tim Deegan <tim@xen.org> Acked-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
master commit: 2823a0c7dfc979db316787e1dd42a8845e5825c0
master date: 2013-07-02 08:49:43 +0200
Jan Beulich [Thu, 11 Jul 2013 12:21:48 +0000 (14:21 +0200)]
x86: don't pass negative time to gtime_to_gtsc() (try 2)
This mostly reverts commit eb60be3d ("x86: don't pass negative time to
gtime_to_gtsc()") and instead corrects __update_vcpu_system_time()'s
handling of this_cpu(cpu_time).stime_local_stamp dating back before the
start of a HVM guest (which would otherwise lead to a negative value
getting passed to gtime_to_gtsc(), causing scale_delta() to produce
meaningless output).
Flushing the value to zero was wrong, and printing a message for
something that can validly happen wasn't very useful either.
Andrew Cooper [Thu, 11 Jul 2013 12:18:57 +0000 (14:18 +0200)]
AMD/intremap: Prevent use of per-device vector maps until irq logic is fixed
XSA-36 changed the default vector map mode from global to per-device. This is
because a global vector map does not prevent one PCI device from impersonating
another and launching a DoS on the system.
However, the per-device vector map logic is broken for devices with multiple
MSI-X vectors, which can either result in a failed ASSERT() or misprogramming
of a guests interrupt remapping tables. The core problem is not trivial to
fix.
In an effort to get AMD systems back to a non-regressed state, introduce a new
type of vector map called per-device-global. This uses per-device vector maps
in the IOMMU, but uses a single used_vector map for the core IRQ logic.
This patch is intended to be removed as soon as the per-device logic is fixed
correctly.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
master commit: f0fe8227624d5c02715ed086867d12cd24f6ff47
master date: 2013-06-27 14:01:18 +0200
Jan Beulich [Tue, 9 Jul 2013 08:04:04 +0000 (10:04 +0200)]
libxl: suppress device assignment to HVM guest when there is no IOMMU
This in effect copies similar logic from xend: While there's no way to
check whether a device is assigned to a particular guest,
XEN_DOMCTL_test_assign_device at least allows checking whether an
IOMMU is there and whether a device has been assign to _some_
guest.
For the time being, this should be enough to cover for the missing
error checking/recovery in other parts of libxl's device assignment
paths.
There remains a (functionality-, but not security-related) race in
that the iommu should be set up earlier, but this is too risky a
change for this stage of the 4.3 release.
This is a security issue, XSA-61.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Tested-by: George Dunlap <george.dunlap@eu.citrix.com> Acked-by: George Dunlap <george.dunlap@eu.citrix.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
master commit: 826eb17271d3c647516d9944c47b0779afedea25
master date: 2013-07-01 15:20:28 +0100
x86/HVM: fix initialization of wallclock time for PVHVM on migration
Call update_domain_wallclock_time on hvm_latch_shinfo_size even if
the bitness of the guest has already been set, this fixes the problem
with the wallclock not being set for PVHVM guests on resume from
migration.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Clean up the resulting code and retain the (slightly adjusted) original
comment.
Zhenguo Wang [Mon, 1 Jul 2013 09:58:26 +0000 (11:58 +0200)]
x86/HVM: fix x2APIC APIC_ID read emulation
APIC and x2APIC have different format for APIC_ID register. Need
translation.
Signed-off-by: Zhenguo Wang <wangzhenguo@huawei.com> Signed-off-by: Xiaowei Yang <xiaowei.yang@huawei.com>
Convert code to use switch(), fixing coding style issue at once, and
use GET_xAPIC_ID() on the value read instead of VLAPIC_ID() (reading
the field again).
In the course of this also properly reject both read and writes on the
non-existing MSR corresponding to APIC_ICR2.
This was never reported to be hit, and we assume to have taken care of
the problem by excluding legacy IRQs from the IRQ move cleanup logic.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
master commit: ac9e298cb4bda0238f50da814b8af2b90dc758a1
master date: 2013-06-10 13:43:03 +0200
Ian Jackson [Thu, 27 Jun 2013 16:25:18 +0000 (17:25 +0100)]
libxl: Restrict permissions on PV console device xenstore nodes
Matthew Daley has observed that the PV console protocol places sensitive host
state into a guest writeable xenstore locations, this includes:
- The pty used to communicate between the console backend daemon and its
client, allowing the guest administrator to read and write arbitrary host
files.
- The output file, allowing the guest administrator to write arbitrary host
files or to target arbitrary qemu chardevs which include sockets, udp, ptr,
pipes etc (see -chardev in qemu(1) for a more complete list).
- The maximum buffer size, allowing the guest administrator to consume more
resources than the host administrator has configured.
- The backend to use (qemu vs xenconsoled), potentially allowing the guest
administrator to confuse host software.
So we arrange to make the sensitive keys in the xenstore frontend directory
read only for the guest. This is safe since the xenstore permissions model,
unlike POSIX directory permissions, does not allow the guest to remove and
recreate a node if it has write access to the containing directory.
There are a few associated wrinkles:
- The primary PV console is "special". It's xenstore node is not under the
usual /devices/ subtree and it does not use the customary xenstore state
machine protocol. Unfortunately its directory is used for other things,
including the vnc-port node, which we do not want the guest to be able to
write to. Rather than trying to track down all the possible secondary uses
of this directory just make it r/o to the guest. All newly created
subdirectories inherit these permissions and so are now safe by default.
- The other serial consoles do use the customary xenstore state machine and
therefore need write access to at least the "protocol" and "state" nodes,
however they may also want to use arbitrary "feature-foo" nodes (although
I'm not aware of any) and therefore we cannot simply lock down the entire
frontend directory. Instead we add support to libxl__device_generic_add for
frontend keys which are explicitly read only and use that to lock down the
sensitive keys.
- Minios' console frontend wants to write the "type" node, which it has no
business doing since this is a host/toolstack level decision. This fails
now that the node has become read only to the PV guest. Since the toolstack
already writes this node just remove the attempt to set it.
This is CVE-2013-2211 / XSA-57
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Conflicts:
tools/libxl/libxl.c (no vtpm, free front_ro on error in
libxl__device_console_add)
In the original patch 7 of the series addressing XSA-45 I mistakenly
took the addition of the call to get_page_light() in alloc_page_type()
to cover two decrements that would happen: One for the PGT_partial bit
that is getting set along with the call, and the other for the page
reference the caller hold (and would be dropping on its error path).
But of course the additional page reference is tied to the PGT_partial
bit, and hence any caller of a function that may leave
->arch.old_guest_table non-NULL for error cleanup purposes has to make
sure a respective page reference gets retained.
Similar issues were then also spotted elsewhere: In effect all callers
of get_page_type_preemptible() need to deal with errors in similar
ways. To make sure error handling can work this way without leaking
page references, a respective assertion gets added to that function.
This is CVE-2013-1432 / XSA-58.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Tested-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Tim Deegan <tim@xen.org>
master commit: 9b167bd2f394f821ae3252d74a15704a4bf91f6d
master date: 2013-06-26 15:32:58 +0200
Ian Jackson [Fri, 14 Jun 2013 15:43:19 +0000 (16:43 +0100)]
libxc: range checks in xc_dom_p2m_host and _guest
These functions take guest pfns and look them up in the p2m. They did
no range checking.
However, some callers, notably xc_dom_boot.c:setup_hypercall_page want
to pass untrusted guest-supplied value(s). It is most convenient to
detect this here and return INVALID_MFN.
This is part of the fix to a security issue, XSA-55.
Changes from Xen 4.2 version of this patch:
* 4.2 lacks dom->rambase_pfn, so don't add/subtract/check it.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian Jackson [Fri, 14 Jun 2013 15:43:19 +0000 (16:43 +0100)]
libxc: check return values from malloc
A sufficiently malformed input to libxc (such as a malformed input ELF
or other guest-controlled data) might cause one of libxc's malloc() to
fail. In this case we need to make sure we don't dereference or do
pointer arithmetic on the result.
Search for all occurrences of \b(m|c|re)alloc in libxc, and all
functions which call them, and add appropriate error checking where
missing.
This includes the functions xc_dom_malloc*, which now print a message
when they fail so that callers don't have to do so.
The function xc_cpuid_to_str wasn't provided with a sane return value
and has a pretty strange API, which now becomes a little stranger.
There are no in-tree callers.
Changes in the Xen 4.2 version of this series:
* No need to fix code relating to ARM.
* No need to fix code relating to superpage support.
* Additionally fix `dom->p2m_host = xc_dom_malloc...' in xc_dom_ia64.c.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian Jackson [Fri, 14 Jun 2013 15:43:19 +0000 (16:43 +0100)]
libxc: check failure of xc_dom_*_to_ptr, xc_map_foreign_range
The return values from xc_dom_*_to_ptr and xc_map_foreign_range are
sometimes dereferenced, or subjected to pointer arithmetic, without
checking whether the relevant function failed and returned NULL.
Add an appropriate error check at every call site.
Changes in the 4.2 backport of this series:
* Fix tools/libxc/xc_dom_x86.c:setup_pgtables_x86_32.
* Fix tools/libxc/xc_dom_ia64.c:start_info_ia64.
* Fix tools/libxc/ia64/xc_ia64_dom_fwloader.c:xc_dom_load_fw_kernel.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian Jackson [Fri, 14 Jun 2013 15:43:19 +0000 (16:43 +0100)]
libxc: Add range checking to xc_dom_binloader
This is a simple binary image loader with its own metadata format.
However, it is too careless with image-supplied values.
Add the following checks:
* That the image is bigger than the metadata table; otherwise the
pointer arithmetic to calculate the metadata table location may
yield undefined and dangerous values.
* When clamping the end of the region to search, that we do not
calculate pointers beyond the end of the image. The C
specification does not permit this and compilers are becoming ever
more determined to miscompile code when they can "prove" various
falsehoods based on assertions from the C spec.
* That the supplied image is big enough for the text we are allegedly
copying from it. Otherwise we might have a read overrun and copy
the results (perhaps a lot of secret data) into the guest.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian Jackson [Fri, 14 Jun 2013 15:43:18 +0000 (16:43 +0100)]
libelf: check loops for running away
Ensure that libelf does not have any loops which can run away
indefinitely even if the input is bogus. (Grepped for \bfor, \bwhile
and \bgoto in libelf and xc_dom_*loader*.c.)
Changes needed:
* elf_note_next uses the note's unchecked alleged length, which might
wrap round. If it does, return ELF_MAX_PTRVAL (0xfff..fff) instead,
which will be beyond the end of the section and so terminate the
caller's loop. Also check that the returned psuedopointer is sane.
* In various loops over section and program headers, check that the
calculated header pointer is still within the image, and quit the
loop if it isn't.
* Some fixed limits to avoid potentially O(image_size^2) loops:
- maximum length of strings: 4K (longer ones ignored totally)
- maximum total number of ELF notes: 65536 (any more are ignored)
* Check that the total program contents (text, data) we copy or
initialise doesn't exceed twice the output image area size.
* Remove an entirely useless loop from elf_xen_parse (!)
* Replace a nested search loop in in xc_dom_load_elf_symtab in
xc_dom_elfloader.c by a precomputation of a bitmap of referenced
symtabs.
We have not changed loops which might, in principle, iterate over the
whole image - even if they might do so one byte at a time with a
nontrivial access check function in the middle.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian Jackson [Fri, 14 Jun 2013 15:43:18 +0000 (16:43 +0100)]
libelf: use only unsigned integers
Signed integers have undesirable undefined behaviours on overflow.
Malicious compilers can turn apparently-correct code into code with
security vulnerabilities etc.
So use only unsigned integers. Exceptions are booleans (which we have
already changed) and error codes.
We _do_ change all the chars which aren't fixed constants from our own
text segment, but not the char*s. This is because it is safe to
access an arbitrary byte through a char*, but not necessarily safe to
convert an arbitrary value to a char.
As a consequence we need to compile libelf with -Wno-pointer-sign.
It is OK to change all the signed integers to unsigned because all the
inequalities in libelf are in contexts where we don't "expect"
negative numbers.
In libelf-dominfo.c:elf_xen_parse we rename a variable "rc" to
"more_notes" as it actually contains a note count derived from the
input image. The "error" return value from elf_xen_parse_notes is
changed from -1 to ~0U.
grepping shows only one occurrence of "PRId" or "%d" or "%ld" in
libelf and xc_dom_elfloader.c (a "%d" which becomes "%u").
This is part of the fix to a security issue, XSA-55.
For those concerned about unintentional functional changes, the
following rune produces a version of the patch which is much smaller
and eliminates only non-functional changes:
Ian Jackson [Fri, 14 Jun 2013 15:43:18 +0000 (16:43 +0100)]
libelf: use C99 bool for booleans
We want to remove uses of "int" because signed integers have
undesirable undefined behaviours on overflow. Malicious compilers can
turn apparently-correct code into code with security vulnerabilities
etc.
In this patch we change all the booleans in libelf to C99 bool,
from <stdbool.h>.
For the one visible libelf boolean in libxc's public interface we
retain the use of int to avoid changing the ABI; libxc converts it to
a bool for consumption by libelf.
It is OK to change all values only ever used as booleans to _Bool
(bool) because conversion from any scalar type to a _Bool works the
same as the boolean test in if() or ?: and is always defined (C99
6.3.1.2). But we do need to check that all these variables really are
only ever used that way. (It is theoretically possible that the old
code truncated some 64-bit values to 32-bit ints which might become
zero depending on the value, which would mean a behavioural change in
this patch, but it seems implausible that treating 0x????????00000000
as false could have been intended.)
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
Ian Jackson [Fri, 14 Jun 2013 15:43:17 +0000 (16:43 +0100)]
libelf: Make all callers call elf_check_broken
This arranges that if the new pointer reference error checking
tripped, we actually get a message about it. In this patch these
messages do not change the actual return values from the various
functions: so pointer reference errors do not prevent loading. This
is for fear that some existing kernels might cause the code to make
these wild references, which would then break, which is not a good
thing in a security patch.
In xen/arch/x86/domain_build.c we have to introduce an "out" label and
change all of the "return rc" beyond the relevant point into "goto
out".
Difference in the 4.2 series, compared to unstable:
* tools/libxc/xc_hvm_build_x86.c:setup_guest and
xen/arch/arm/kernel.c:kernel_try_elf_prepare have different
error handling in 4.2 to unstable; patch adjusted accordingly.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
xen-unstable version Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>
Ian Jackson [Fri, 14 Jun 2013 15:43:17 +0000 (16:43 +0100)]
libelf: Check pointer references in elf_is_elfbinary
elf_is_elfbinary didn't take a length parameter and could potentially
access out of range when provided with a very short image.
We only need to check the size is enough for the actual dereference in
elf_is_elfbinary; callers are just using it to check the magic number
and do their own checks (usually via the new elf_ptrval system) before
dereferencing other parts of the header.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Ian Jackson [Fri, 14 Jun 2013 15:43:17 +0000 (16:43 +0100)]
libelf: check all pointer accesses
We change the ELF_PTRVAL and ELF_HANDLE types and associated macros:
* PTRVAL becomes a uintptr_t, for which we provide a typedef
elf_ptrval. This means no arithmetic done on it can overflow so
the compiler cannot do any malicious invalid pointer arithmetic
"optimisations". It also means that any places where we
dereference one of these pointers without using the appropriate
macros or functions become a compilation error.
So we can be sure that we won't miss any memory accesses.
All the PTRVAL variables were previously void* or char*, so
the actual address calculations are unchanged.
* ELF_HANDLE becomes a union, one half of which keeps the pointer
value and the other half of which is just there to record the
type.
The new type is not a pointer type so there can be no address
calculations on it whose meaning would change. Every assignment or
access has to go through one of our macros.
* The distinction between const and non-const pointers and char*s
and void*s in libelf goes away. This was not important (and
anyway libelf tended to cast away const in various places).
* The fields elf->image and elf->dest are renamed. That proves
that we haven't missed any unchecked uses of these actual
pointer values.
* The caller may fill in elf->caller_xdest_base and _size to
specify another range of memory which is safe for libelf to
access, besides the input and output images.
* When accesses fail due to being out of range, we mark the elf
"broken". This will be checked and used for diagnostics in
a following patch.
We do not check for write accesses to the input image. This is
because libelf actually does this in a number of places. So we
simply permit that.
* Each caller of libelf which used to set dest now sets
dest_base and dest_size.
* In xc_dom_load_elf_symtab we provide a new actual-pointer
value hdr_ptr which we get from mapping the guest's kernel
area and use (checking carefully) as the caller_xdest area.
* The STAR(h) macro in libelf-dominfo.c now uses elf_access_unsigned.
* elf-init uses the new elf_uval_3264 accessor to access the 32-bit
fields, rather than an unchecked field access (ie, unchecked
pointer access).
* elf_uval has been reworked to use elf_uval_3264. Both of these
macros are essentially new in this patch (although they are derived
from the old elf_uval) and need careful review.
* ELF_ADVANCE_DEST is now safe in the sense that you can use it to
chop parts off the front of the dest area but if you chop more than
is available, the dest area is simply set to be empty, preventing
future accesses.
* We introduce some #defines for memcpy, memset, memmove and strcpy:
- We provide elf_memcpy_safe and elf_memset_safe which take
PTRVALs and do checking on the supplied pointers.
- Users inside libelf must all be changed to either
elf_mem*_unchecked (which are just like mem*), or
elf_mem*_safe (which take PTRVALs) and are checked. Any
unchanged call sites become compilation errors.
* We do _not_ at this time fix elf_access_unsigned so that it doesn't
make unaligned accesses. We hope that unaligned accesses are OK on
every supported architecture. But it does check the supplied
pointer for validity.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian Jackson [Fri, 14 Jun 2013 15:43:17 +0000 (16:43 +0100)]
libelf: check nul-terminated strings properly
It is not safe to simply take pointers into the ELF and use them as C
pointers. They might not be properly nul-terminated (and the pointers
might be wild).
So we are going to introduce a new function elf_strval for safely
getting strings. This will check that the addresses are in range and
that there is a proper nul-terminated string. Of course it might
discover that there isn't. In that case, it will be made to fail.
This means that elf_note_name might fail, too.
For the benefit of call sites which are just going to pass the value
to a printf-like function, we provide elf_strfmt which returns
"(invalid)" on failure rather than NULL.
In this patch we introduce dummy definitions of these functions. We
introduce calls to elf_strval and elf_strfmt everywhere, and update
all the call sites with appropriate error checking.
There is not yet any semantic change, since before this patch all the
places where we introduce elf_strval dereferenced the value anyway, so
it mustn't have been NULL.
In future patches, when elf_strval is made able return NULL, when it
does so it will mark the elf "broken" so that an appropriate
diagnostic can be printed.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Ian Jackson [Fri, 14 Jun 2013 15:43:17 +0000 (16:43 +0100)]
libelf: introduce macros for memory access and pointer handling
We introduce a collection of macros which abstract away all the
pointer arithmetic and dereferences used for accessing the input ELF
and the output area(s). We use the new macros everywhere.
For now, these macros are semantically identical to the code they
replace, so this patch has no functional change.
elf_is_elfbinary is an exception: since it doesn't take an elf*, we
need to handle it differently. In a future patch we will change it to
take, and check, a length parameter. For now we just mark it with a
fixme.
That this patch has no functional change can be verified as follows:
0. Copy the scripts "comparison-generate" and "function-filter"
out of this commit message.
1. Check out the tree before this patch.
2. Run the script ../comparison-generate .... ../before
3. Check out the tree after this patch.
4. Run the script ../comparison-generate .... ../after
5. diff --exclude=\*.[soi] -ruN before/ after/ |less
Expect these differences:
* stubdom/zlib-x86_64/ztest*.s2
The filename of this test file apparently contains the pid.
* xen/common/version.s2
The xen build timestamp appears in two diff hunks.
Verification that this is all that's needed:
In a completely built xen.git,
find * -name .*.d -type f | xargs grep -l libelf\.h
Expect results in:
xen/arch/x86: Checked above.
tools/libxc: Checked above.
tools/xcutils/readnotes: Checked above.
tools/xenstore: Checked above.
xen/common/libelf:
This is the build for the hypervisor; checked in B above.
stubdom:
We have one stubdom which reads ELFs using our libelf,
pvgrub, which is checked above.
I have not done this verification for ARM.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
-8<- comparison-generate -8<-
#!/bin/bash
# usage:
# cd xen.git
# .../comparison-generate OUR-CONFIG BUILD-RUNE-PREFIX ../before|../after
# eg:
# .../comparison-generate ~/work/.config 'schroot -pc64 --' ../before
set -ex
test $# = 3 || need-exactly-three-arguments
our_config=$1
build_rune_prefix=$2
result_dir=$3
git clean -x -d -f
cp "$our_config" .
cat <<END >>.config
debug_symbols=n
CFLAGS += -save-temps
END
if [ -f ./configure ]; then
$build_rune_prefix ./configure
fi
$build_rune_prefix make -C xen
$build_rune_prefix make -C tools/include
$build_rune_prefix make -C stubdom grub
$build_rune_prefix make -C tools/libxc
$build_rune_prefix make -C tools/xenstore
$build_rune_prefix make -C tools/xcutils
rm -rf "$result_dir"
mkdir "$result_dir"
set +x
for f in `find xen tools stubdom -name \*.[soi]`; do
mkdir -p "$result_dir"/`dirname $f`
cp $f "$result_dir"/${f}
case $f in
*.s)
../function-filter <$f >"$result_dir"/${f}2
;;
esac
done
echo ok.
-8<-
-8<- function-filter -8<-
#!/usr/bin/perl -w
# function-filter
# script for massaging gcc-generated labels to be consistent
use strict;
our @lines;
my $sedderybody = "sub seddery () {\n";
while (<>) {
push @lines, $_;
if (m/^(__FUNCTION__|__func__)\.(\d+)\:/) {
$sedderybody .= " s/\\b$1\\.$2\\b/__XSA55MANGLED__$1.$./g;\n";
}
}
$sedderybody .= "}\n1;\n";
eval $sedderybody or die $@;
foreach (@lines) {
seddery();
print or die $!;
}
-8<-
Ian Jackson [Fri, 14 Jun 2013 15:43:16 +0000 (16:43 +0100)]
libelf/xc_dom_load_elf_symtab: Do not use "syms" uninitialised
xc_dom_load_elf_symtab (with load==0) calls elf_round_up, but it
mistakenly used the uninitialised variable "syms" when calculating
dom->bsd_symtab_start. This should be a reference to "elf".
This change might have the effect of rounding the value differently.
Previously if the uninitialised value (a single byte on the stack) was
ELFCLASS64 (ie, 2), the alignment would be to 8 bytes, otherwise to 4.
However, the value is calculated from dom->kernel_seg.vend so this
could only make a difference if that value wasn't already aligned to 8
bytes.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Ian Jackson [Fri, 14 Jun 2013 15:43:16 +0000 (16:43 +0100)]
libelf: move include of <asm/guest_access.h> to top of file
libelf-loader.c #includes <asm/guest_access.h>, when being compiled
for Xen. Currently it does this in the middle of the file.
Move this #include to the top of the file, before libelf-private.h.
This is necessary because in forthcoming patches we will introduce
private #defines of memcpy etc. which would interfere with definitions
in headers #included from guest_access.h.
No semantic or functional change in this patch.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Ian Jackson [Fri, 14 Jun 2013 15:43:16 +0000 (16:43 +0100)]
libelf: add `struct elf_binary*' parameter to elf_load_image
The meat of this function is going to need a copy of the elf pointer,
in forthcoming patches.
No functional change in this patch.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Ian Jackson [Fri, 14 Jun 2013 15:43:16 +0000 (16:43 +0100)]
libxc: Fix range checking in xc_dom_pfn_to_ptr etc.
* Ensure that xc_dom_pfn_to_ptr (when called with count==0) does not
return a previously-allocated block which is entirely before the
requested pfn (!)
* Provide a version of xc_dom_pfn_to_ptr, xc_dom_pfn_to_ptr_retcount,
which provides the length of the mapped region via an out parameter.
* Change xc_dom_vaddr_to_ptr to always provide the length of the
mapped region and change the call site in xc_dom_binloader.c to
check it. The call site in xc_dom_load_elf_symtab will be corrected
in a forthcoming patch, and for now ignores the returned length.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian Jackson [Fri, 14 Jun 2013 15:43:16 +0000 (16:43 +0100)]
libxc: introduce xc_dom_seg_to_ptr_pages
Provide a version of xc_dom_seg_to_ptr which returns the number of
guest pages it has actually mapped. This is useful for callers who
want to do range checking; we will use this later in this series.
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Ian Jackson [Fri, 14 Jun 2013 15:43:15 +0000 (16:43 +0100)]
libelf: abolish libelf-relocate.c
This file is not actually used. It's not built in Xen's instance of
libelf; in libxc's it's built but nothing in it is called. Do not
compile it in libxc, and delete it.
This reduces the amount of work we need to do in forthcoming patches
to libelf (particularly since as libelf-relocate.c is not used it is
probably full of bugs).
This is part of the fix to a security issue, XSA-55.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Roger Pau Monné [Thu, 13 Jun 2013 09:26:53 +0000 (11:26 +0200)]
x86/vtsc: update vcpu_time in hvm_set_guest_time
When using a vtsc, hvm_set_guest_time changes hvm_vcpu.stime_offset,
which is used in the vcpu time structure to calculate the
tsc_timestamp, so after updating stime_offset we need to propagate the
change to vcpu_time in order for the guest to get the right time if
using the PV clock.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>
master commit: 32c864a35ece2c24a336d183869a546798a4b241
master date: 2013-06-05 10:03:08 +0200
Jan Beulich [Thu, 13 Jun 2013 09:23:56 +0000 (11:23 +0200)]
x86: fix XCR0 handling
- both VMX and SVM ignored the ECX input to XSETBV
- both SVM and VMX used the full 64-bit RAX when calculating the input
mask to XSETBV
- faults on XSETBV did not get recovered from
Also consolidate the handling for PV and HVM into a single function,
and make the per-CPU variable "xcr0" static to xstate.c.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Keir Fraser <keir@xen.org> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
master commit: 10b2b21a241795394637167bd4b076f2de17741f
master date: 2013-06-04 17:25:41 +0200
Jan Beulich [Thu, 13 Jun 2013 09:20:40 +0000 (11:20 +0200)]
x86: preserve FPU selectors for 32-bit guest code
Doing {,F}X{SAVE,RSTOR} unconditionally with 64-bit operand size leads
to the selector values associated with the last instruction/data
pointers getting lost. This, besides being inconsistent and not
compatible with native hardware behavior especially for 32-bit guests,
leads to bug checks in 32-bit Windows when running with "Driver
verifier" (see e.g. http://support.microsoft.com/kb/244617).
In a first try I made the code figure out the current guest mode, but
that has the disadvantage of only taking care of the issue when the
guest executes in the mode for which the state currently is (i.e.
namely not in the case of a 64-bit guest running a 32-bit application,
but being in kernle [64-bit] mode).
The solution presented here is to conditionally execute an auxiliary
FNSTENV and use the selectors from there.
In either case the determined word size gets stored in the last byte
of the FPU/SSE save area, which is available for software use (and I
verified is being cleared to zero by all versions of Xen, i.e. will not
present a problem when migrating guests from older to newer hosts), and
evaluated for determining the operand size {,F}XRSTOR is to be issued
with.
Note that I did check whether using a second FXSAVE or a partial second
XSAVE would be faster than FNSTENV - neither on my Westmere (FXSAVE)
nor on my Romley (XSAVE) they are.
I was really tempted to use branches into the middle of instructions
(i.e. past the REX64 prefixes) here, as that would have allowed to
collapse the otherwise identical fault recovery blocks. I stayed away
from doing so just because I expect others to dislike such slightly
subtle/tricky code.
Reported-by: Ben Guthro <Benjamin.Guthro@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Keir Fraser <keir@xen.org> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
master commit: 10f969150025498fe27d985f9021a68f8c7acc31
master date: 2013-06-04 17:23:11 +0200
When booting Xen on VMware ESX 5.1 and Workstation 9, you hit a GPF
during MCE initialization. The culprit is line 631 in
set_poll_bankmask():
bitmap_copy(mb->bank_map, mca_allbanks->bank_map, nr_mce_banks);
What is happening is that in mca_cap_init(), nr_mce_banks is being set
to 0. This causes the allocation of bank_map to be set to
ZERO_BLOCK_PTR which is the return value for zero-size allocation by
xzalloc_array()/_xmalloc(). This results in the bitmap_copy() to fail
disastrously. The following patch fixes this issue.
Signed-off-by: Aravindh Puthiyaparambil <aravindp@cisco.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Christoph Egger <chegger@amazon.de>
master commit: 5cffb77c4072fa5b46700a2dbb3e46c5a54eba6d
master date: 2013-06-03 15:42:46 +0200
Jan Beulich [Thu, 13 Jun 2013 09:17:46 +0000 (11:17 +0200)]
x86: fix ordering of operations in destroy_irq()
The fix for XSA-36, switching the default of vector map management to
be per-device, exposed more readily a problem with the cleanup of these
vector maps: dynamic_irq_cleanup() clearing desc->arch.used_vectors
keeps the subsequently invoked clear_irq_vector() from clearing the
bits for both the in-use and a possibly still outstanding old vector.
Fix this by folding dynamic_irq_cleanup() into destroy_irq(), which was
its only caller, deferring the clearing of the vector map pointer until
after clear_irq_vector().
Once at it, also defer resetting of desc->handler until after the loop
around smp_mb() checking for IRQ_INPROGRESS to be clear, fixing a
(mostly theoretical) issue with the intercation with do_IRQ(): If we
don't defer the pointer reset, do_IRQ() could, for non-guest IRQs, call
->ack() and ->end() with different ->handler pointers, potentially
leading to an IRQ remaining un-acked. The issue is mostly theoretical
because non-guest IRQs are subject to destroy_irq() only on (boot time)
error paths.
As to the changed locking: Invoking clear_irq_vector() with desc->lock
held is okay because vector_lock already nests inside desc->lock (proven
by set_desc_affinity(), which takes vector_lock and gets called from
various desc->handler->ack implementations, getting invoked with
desc->lock held).
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Keir Fraser <keir@xen.org> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
master commit: 43427e65ccfea0c6dc0232f358287e6cc616b507
master date: 2013-05-31 08:35:22 +0200
Jan Beulich [Tue, 4 Jun 2013 07:36:32 +0000 (09:36 +0200)]
x86/xsave: properly check guest input to XSETBV
Other than the HVM emulation path, the PV case so far failed to check
that YMM state requires SSE state to be enabled, allowing for a #GP to
occur upon passing the inputs to XSETBV inside the hypervisor.
Jan Beulich [Tue, 4 Jun 2013 07:35:16 +0000 (09:35 +0200)]
x86/xsave: recover from faults on XRSTOR
Just like FXRSTOR, XRSTOR can raise #GP if bad content is being passed
to it in the memory block (i.e. aspects not under the control of the
hypervisor, other than e.g. proper alignment of the block).
Also correct the comment explaining why FXRSTOR needs exception
recovery code to not wrongly state that this can only be a result of
the control tools passing a bad image.
Jan Beulich [Tue, 4 Jun 2013 07:33:29 +0000 (09:33 +0200)]
x86/xsave: fix information leak on AMD CPUs
Just like for FXSAVE/FXRSTOR, XSAVE/XRSTOR also don't save/restore the
last instruction and operand pointers as well as the last opcode if
there's no pending unmasked exception (see CVE-2006-1056 and commit
9747:4d667a139318).
While the FXSR solution sits in the save path, I prefer to have this in
the restore path because there the handling is simpler (namely in the
context of the pending changes to properly save the selector values for
32-bit guest code).
Also this is using FFREE instead of EMMS, as it doesn't seem unlikely
that in the future we may see CPUs with x87 and SSE/AVX but no MMX
support. The goal here anyway is just to avoid an FPU stack overflow.
I would have preferred to use FFREEP instead of FFREE (freeing two
stack slots at once), but AMD doesn't document that instruction.
Petr Matousek [Fri, 31 May 2013 10:24:18 +0000 (12:24 +0200)]
libxc: limit cpu values when setting vcpu affinity
When support for pinning more than 64 cpus was added, check for cpu
out-of-range values was removed. This can lead to subsequent
out-of-bounds cpumap array accesses in case the cpu number is higher
than the actual count.
Jan Beulich [Fri, 31 May 2013 10:23:30 +0000 (12:23 +0200)]
x86: fix boot time APIC mode detection
current_cpu_data becomes valid only relatively late in the boot
process, so looking there for a particular feature early in the game
would generally give the appearance of the feature being unavailable.
Getting this wrong means that at kexec time the system would get
returned to xAPIC mode, causing disconnect_bsp_APIC() to try to access
the APIC page, which on systems with x2APIC pre-enabled will never get
set up.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Keir Fraser <keir@xen.org> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 234c4dde2fd4f1182fe1a6bea6bced83fe363007
master date: 2013-05-23 13:08:32 +0200
Andrew Cooper [Thu, 23 May 2013 08:16:28 +0000 (10:16 +0200)]
AMD/iommu: SR56x0 Erratum 64 - Reset all head & tail pointers
Reference at time of patch:
http://support.amd.com/us/ChipsetMotherboard_TechDocs/46303.pdf
Erratum 64 states that the head and tail pointers for the Command buffer and
Event log are only reset on a cold boot, not a warm boot.
While the erratum is limited to systems using SR56xx chipsets (such as Family
10h CPUs), resetting the pointers is a sensible action in all cases, including
the PPR log for consistency.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Keir Fraser <keir@xen.org> Acked-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
master commit: 6d243308e1d75f866679db226159c797d6c83aad
master date: 2013-05-22 15:26:52 +0200
Jan Beulich [Thu, 23 May 2013 08:15:55 +0000 (10:15 +0200)]
tools: fix dependency file generation
There is a small set of places where files in subdirectories get
compiled from the parent directory. Dependency file wise this is no
problem as long as the files use names distinct without regard to the
directories they sit in, and tools/console/ violates this (in having
two main.c files). Hence we need to avoid losing the directory name,
both to ensure the two compiler instances don't simultaneously write
to the same file (happening of which is what triggered me looking
into this) and to guarantee dependencies for all files will be seen
by make on an incremental rebuild.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Ian Campbell <ian.campbell@citrix.com>
master commit: 4d788e164d6556d931bc3e0a69e36b8cf7280794
master date: 2013-05-21 10:16:30 +0200
Jan Beulich [Thu, 23 May 2013 08:13:18 +0000 (10:13 +0200)]
fix XSA-46 regression with xend/xm
The hypervisor side changes for XSA-46 require the tool stack to now
always map the guest pIRQ before granting access permission to the
underlying host IRQ (GSI). This in particular requires that pciif.py
no longer can skip this step (assuming qemu would do it) for HVM
guests.
This in turn exposes, however, an inconsistency between xend and qemu:
The former wants to always establish 1:1 mappings between pIRQ and host
IRQ (for non-MSI only of course), while the latter always wants to
allocate an arbitrary mapping. Since the whole tool stack obviously
should always agree on the mapping model, make libxc enforce the 1:1
mapping as the more natural one (as well as being the one that allows
for easier debugging, since there no need to find out the extra
mapping). Users of libxc that want to establish a particular (rather
than an allocated) mapping are still free to do so, as well as tool
stacks not based on libxc wanting to implement an allocation based
model (which is why it's not the hypervisor that's being changed to
enforce either model).
Since libxl, like xend, already uses a 1:1 model, it's unaffected by
the libxc change (and it being unaffected by the original hypervisor
side changes is - afaict - simply due to qemu getting spawned at a
later point in time compared to the xend event flow).
Signed-off-by: Jan Beulich <jbeulich@suse.com> Tested-by: Andreas Falck <falck.andreas.lists@gmail.com> (on 4.1) Tested-by: Gordan Bobic <gordan@bobich.net> (on 4.2) Acked-by: Ian Campbell <ian.campbell@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 934a5253d932b6f67fe40fc48975a2b0117e4cce
master date: 2013-05-21 11:32:34 +0200
Tim Deegan [Thu, 23 May 2013 08:11:12 +0000 (10:11 +0200)]
x86/hvm: avoid p2m lookups for vlapic accesses.
The LAPIC base address is a known GFN, so we can skip looking up the
p2m: we know it should be handled as emulated MMIO. That helps
performance in older Windows OSes, which make a _lot_ of TPR accesses.
This will change the behaviour of any OS that maps other
memory/devices at its LAPIC address; the new behaviour (the LAPIC
mapping always wins) is closer to actual hardware behaviour.
Signed-off-by: Tim Deegan <tim@xen.org> Acked-by: Jan Beulich <jbeulich@suse.com>
master commit: 5d43891bf4002b754cd90d83e91d9190e8c8b9d0
master date: 2013-05-16 12:05:25 +0100
Jan Beulich [Thu, 23 May 2013 08:09:17 +0000 (10:09 +0200)]
x86/IO-APIC: fix guest RTE write corner cases
This fixes two regressions from c/s 20143:a7de5bd776ca ("x86: Make the
hypercall PHYSDEVOP_alloc_irq_vector hypercall dummy"):
For one, IRQs that had their vector set up by Xen internally without a
handler ever having got set (e.g. via "com<n>=..." without a matching
consumer option like "console=com<n>") would wrongly call
add_pin_to_irq() here, triggering the BUG_ON() in that function.
Second, when assign_irq_vector() fails this addition to irq_2_pin[]
needs to be undone.
In the context of this I'm also surprised that the irq_2_pin[]
manipulations here occur without any lock, i.e. rely on Dom0 to do
some sort of serialization.
Update return codes of wrmsr_hypervisor_regs, update callers to deal
with the new return codes:
0: not handled
1: handled
-EAGAIN: retry
Currently wrmsr_hypervisor_regs will not return the following error, it
will be added in a separate patch:
-EINVAL: error during handling
Also update the gdprintk to handle a page value of NULL to avoid
printing a bogus MFN value. Update also computing of MSR value in
gdprintk, the idx was always zero.
Ben Guthro [Thu, 23 May 2013 08:06:34 +0000 (10:06 +0200)]
ns16550: delay resume until dom0 ACPI has a chance to run
Check for ioport access, before fully resuming operation, to avoid
spinning in __ns16550_poll when reading the LSR register returns 0xFF
on failing ioport access.
On some systems (like Lenovo T410, and some HP machines of similar vintage)
there is a SuperIO card that provides this legacy ioport on the LPC bus.
In this case, we need to wait for dom0's ACPI processing to run the proper
AML to re-initialize the chip, before we can use the card again.
This may cause a small amount of garbage to be written to the serial log
while we wait patiently for that AML to be executed.
This implementation limits the number of retries, to avoid a situation
where we keep trying over and over again, in the case of some other failure
on the ioport.
The current code allows the PVHVM guest to make this hypercall.
But for PVHVM guest it always returns -EINVAL (-22) for Xen 4.2
and above. Xen 4.1 and earlier worked.
The reason is that the check in map_vcpu_info would fail
at:
if ( v->arch.vcpu_info_mfn != INVALID_MFN )
The reason is that the vcpu_info_mfn for PVHVM guests ends up by
defualt with the value of zero (introduced by c/s 23143).
The code in vcpu_initialise which initialized vcpu_info_mfn to a
valid value (INVALID_MFN), would never be called for PVHVM:
Jan Beulich [Fri, 3 May 2013 07:42:15 +0000 (09:42 +0200)]
x86: cleanup after making various page table manipulation operations preemptible
This drops the "preemptible" parameters from various functions where
now they can't (or shouldn't, validated by assertions) be run in non-
preemptible mode anymore, to prove that manipulations of at least L3
and L4 page tables and page table entries are now always preemptible,
i.e. the earlier patches actually fulfill their purpose of fixing the
resulting security issue.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Tim Deegan <tim@xen.org>
master commit: b965b31a6bce8c37e67e525fae6da0e2f26d6b2e
master date: 2013-05-02 17:04:14 +0200
Jan Beulich [Thu, 2 May 2013 15:19:41 +0000 (17:19 +0200)]
VT-d: don't permit SVT_NO_VERIFY entries for known device types
Only in cases where we don't know what to do we should leave the IRTE
blank (suppressing all validation), but we should always log a warning
in those cases (as being insecure).
Jan Beulich [Thu, 2 May 2013 15:19:01 +0000 (17:19 +0200)]
x86: make page table handling error paths preemptible
... as they may take significant amounts of time.
This requires cloning the tweaked continuation logic from
do_mmuext_op() to do_mmu_update().
Note that in mod_l[34]_entry() a negative "preemptible" value gets
passed to put_page_from_l[34]e() now, telling the callee to store the
respective page in current->arch.old_guest_table (for a hypercall
continuation to pick up), rather than carrying out the put right away.
This is going to be made a little more explicit by a subsequent cleanup
patch.
This is part of CVE-2013-1918 / XSA-45.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Tim Deegan <tim@xen.org>
master commit: b8efae696c9a2d46e91fa0eda739427efc16c250
master date: 2013-05-02 16:39:37 +0200
Jan Beulich [Thu, 2 May 2013 15:18:28 +0000 (17:18 +0200)]
x86: make page table unpinning preemptible
... as it may take significant amounts of time.
Since we can't re-invoke the operation in a second attempt, the
continuation logic must be slightly tweaked so that we make sure
do_mmuext_op() gets run one more time even when the preempted unpin
operation was the last one in a batch.
This is part of CVE-2013-1918 / XSA-45.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Tim Deegan <tim@xen.org>
master commit: a3e049f8e86fe18e3b87f18dc0c7be665026fd9f
master date: 2013-05-02 16:39:06 +0200
Jan Beulich [Thu, 2 May 2013 15:14:06 +0000 (17:14 +0200)]
x86: make vcpu_destroy_pagetables() preemptible
... as it may take significant amounts of time.
The function, being moved to mm.c as the better home for it anyway, and
to avoid having to make a new helper function there non-static, is
given a "preemptible" parameter temporarily (until, in a subsequent
patch, its other caller is also being made capable of dealing with
preemption).
This is part of CVE-2013-1918 / XSA-45.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Tim Deegan <tim@xen.org>
master commit: 6cdc9be2a5f2a87b4504404fbf648d16d9503c19
master date: 2013-05-02 16:34:21 +0200
Ben Guthro [Mon, 29 Apr 2013 14:08:16 +0000 (16:08 +0200)]
x86/S3: Fix cpu pool scheduling after suspend/resume
This review is another S3 scheduler problem with the system_state
variable introduced with the following changeset:
http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=269f543ea750ed567d18f2e819e5d5ce58eda5c5
Specifically, the cpu_callback function that takes the CPU down during
suspend, and back up during resume. We were seeing situations where,
after S3, only CPU0 was in cpupool0. Guest performance suffered
greatly, since all vcpus were only on a single pcpu. Guests under high
CPU load showed the problem much more quickly than an idle guest.
Removing this if condition forces the CPUs to go through the expected
online/offline state, and be properly scheduled after S3.
This also includes a necessary partial change proposed earlier by
Tomasz Wroblewski here:
http://lists.xen.org/archives/html/xen-devel/2013-01/msg02206.html
It should also resolve the issues discussed in this thread:
http://lists.xen.org/archives/html/xen-devel/2012-11/msg01801.html
Ian Jackson [Thu, 18 Apr 2013 16:42:04 +0000 (17:42 +0100)]
libxl: Fix SEGV in network-attach
When "device/vif" directory exists but is empty l!=NULL, but nb==0, so
l[nb-1] is invalid. Add missing check.
Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Jan Beulich [Thu, 18 Apr 2013 14:19:51 +0000 (16:19 +0200)]
x86: fix various issues with handling guest IRQs
- properly revoke IRQ access in map_domain_pirq() error path
- don't permit replacing an in use IRQ
- don't accept inputs in the GSI range for MAP_PIRQ_TYPE_MSI
- track IRQ access permission in host IRQ terms, not guest IRQ ones
(and with that, also disallow Dom0 access to IRQ0)
Jan Beulich [Thu, 18 Apr 2013 14:18:42 +0000 (16:18 +0200)]
x86: clear EFLAGS.NT in SYSENTER entry path
... as it causes problems if we happen to exit back via IRET: In the
course of trying to handle the fault, the hypervisor creates a stack
frame by hand, and uses PUSHFQ to set the respective EFLAGS field, but
expects to be able to IRET through that stack frame to the second
portion of the fixup code (which causes a #GP due to the stored EFLAGS
having NT set).
And even if this worked (e.g if we cleared NT in that path), it would
then (through the fail safe callback) cause a #GP in the guest with the
SYSENTER handler's first instruction as the source, which in turn would
allow guest user mode code to crash the guest kernel.
Inject a #GP on the fake (NULL) address of the SYSENTER instruction
instead, just like in the case where the guest kernel didn't register
a corresponding entry point.
This is CVE-2013-1917 / XSA-44.
Reported-by: Andrew Cooper <andrew.cooper3@citirx.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Tested-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: fdac9515607b757c044e7ef0d61b1453ef999b08
master date: 2013-04-18 16:00:35 +0200
On the crash path in nmi_shootdown_cpus(), we shut down the IOMMU, then
disable the IOAPIC.
On systems which support interrupt remapping, the variable iommu_intremap
remains set, meaning that disable_IO_APIC() issues interrupt remapping
invalidate requests.
IOAPIC interrupt remapping used to be conditional on iommu_enabled, but is now
conditional on iommu_intremap, following the above changeset.
This behaviour can be fixed by also indicating that interrupt remapping is not
enabled after shutting down the IOMMU.
tools/blktap2: Handle read/write interrupts in blktap2 control plane.
The following patch:
tools: Retry blktap2 tapdisk message on interrupt.
Addressed a long standing regression with the blktap2 control
plane. An interruption of the select system call would
prematurely terminate the message sequence needed to properly
shutdown a blktap2 tapdisk instance.
Ian Jackson correctly noted that the read and write systems calls
responsible for receiving and sending the control messages could
also return EINTR resulting in similar effects. While this
regression was not noted in field testing this patch adds support
to re-start the calls to provide a technically complete
implementation of control plane management in the presence of
signals.
Signed-off-by: Dr. Greg Wettstein <xen@wind.enjellic.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit a5c800142cfc82159fcb85b47116cf296caebcc5)
Ian Jackson [Mon, 15 Apr 2013 17:04:35 +0000 (18:04 +0100)]
libxl: don't launch more than one tapdisk process for each disk
When adding a disk don't launch multiple tapdisk instances for the
same disk, if transaction fails in device_disk_add reuse the same
tapdisk for further tries instead of creating a new instance each
time a transaction fails.
Reported-by: Darren Shepherd <darren.s.shepherd@gmail.com> Signed-off-by: Roger Pau Monne <roger.pau@citrix.com> Tested-by: Darren Shepherd <darren.s.shepherd@gmail.com> Backport-requested-by: Pasi Karkkainen <pasik@iki.fi>
(cherry picked from commit ec398660e89ca18bb8d061d5047d682bd383778a)
Conflicts:
tools/libxl/libxl.c Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
tools: Retry blktap2 tapdisk message on interrupt.
Re-start blktap2 IPC select call on interrupt.
We hunted this miserable bug for a long time.
The teardown of a blktap2 tapdisk instance is being carried out
inconsistently up to and including the 4.2.1 release. The
problem appears to be a classic 'Heisenbug' which disappears if a
single function call is added to the tapdisk shutdown path. It
is likely this bug has been in existence for the life of the
blktap2 code.
Control messages to manipulate a tapdisk instance are sent over a
UNIX domain socket. A select call is used on both the read and
write paths to wait on I/O and to set a timeout for the
transmission and reception of the control plane messages.
The existing code fails receipt or transmission of the control message
on any type of error return from the select call. The xl control
process receives an interrupt while waiting in the select call which
in turn causes an error return with SIGINT as the return code.
This prematurely terminates the teardown of the tapdisk instance
leaving it in various states of shutdown. Since multiple messages
are needed to implement a full teardown the tapdisk instance can be
left in various states ranging from fully connected to only the minor
being left allocated.
The fix is straight forward. Check the return code from the
select call and re-try read or write of the control message if
errno is sent to EINTR. The problem manifests itself in the read
path but there appears to be little reason to not add the fix to
the write path as well. Both paths appear to be cut-and-paste
copies of each other.
Signed-off-by: Dr. Greg Wettstein <greg@enjellic.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
(cherry picked from commit 6cffb2b469a55032a2900ccb8776c0082f346758)
Ian Jackson [Tue, 9 Apr 2013 14:44:07 +0000 (15:44 +0100)]
libxl: run libxl__arch_domain_create() much earlier.
Among other things, arch_domain_create() sets the shadow(/hap/p2m)
memory allocation, which must happen after vcpus are assigned (or the
shadow op will fail) but before memory is allocated (or we might run
out of p2m memory).
libxl__build_pre(), which already sets similar things like maxmem,
semes like a reasonable spot for it. That needed a bit of plumbing to
get the right datastructure from the caller.
As a side-effect, the return code from libxl__arch_domain_create() is
no longer ignored.
This bug was analysed in:
From: "Jan Beulich" <JBeulich@xxxxxxxx>
"Re: [Xen-devel] [xen-unstable test] 16788: regressions - FAIL"
Date: Mon, 04 Mar 2013 16:34:53 +0000
http://lists.xen.org/archives/html/xen-devel/2013-03/msg00191.html
Reported-by: Jan Beulich <JBeulich@suse.com> Signed-off-by: Tim Deegan <tim@xen.org> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com>
(Cherry-picked from 650354dbc2626b643c12873275ca67782f1382c8.)
Conflicts:
tools/libxl/libxl_dom.c Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Tim Deegan [Tue, 9 Apr 2013 14:07:23 +0000 (16:07 +0200)]
x86/mm/shadow: spurious warning when unmapping xenheap pages.
Xenheap pages will always have an extra typecount, taken in
share_xen_page_with_guest(), which doesn't come from a shadow PTE.
Adjust the warning in sh_remove_all_mappings() to account for it.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Tim Deegan <tim@xen.org> Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: cfc515dabe91e3d6c690c68c6a669d6d77fb7ac4
master date: 2013-04-04 10:14:30 +0100
disabled the SMEP bit if a guest VCPU was using HAP and was not
in paging mode. However I could observe VCPUs getting stuck in
the trampoline after the following patch in the Linux kernel
changed the way CR4 gets set up:
x86, realmode: read cr4 and EFER from kernel for 64-bit trampoline
The change will set CR4 from already set flags which includes the
SMEP bit. On bare metal this does not matter as the CPU is in non-
paging mode at that time. But Xen seems to use the emulated non-
paging mode regardless of HAP (I verified that on the guests I was
seeing the issue, HAP was not used).
Therefor it seems right to unset the SMEP bit for a VCPU that is
not in paging-mode, regardless of its HAP usage.
Ben Guthro [Tue, 9 Apr 2013 14:05:52 +0000 (16:05 +0200)]
x86/S3: Restore broken vcpu affinity on resume
When in SYS_STATE_suspend, and going through the cpu_disable_scheduler
path, save a copy of the current cpu affinity, and mark a flag to
restore it later.
Later, in the resume process, when enabling nonboot cpus restore these
affinities.
Signed-off-by: Ben Guthro <benjamin.guthro@citrix.com> Acked-by: George Dunlap <george.dunlap@eu.citrix.com> Acked-by: Keir Fraser <keir@xen.org>
master commit: 41e71c2607e036f1ac00df898b8f4acb2d4df7ee
master date: 2013-04-02 09:52:32 +0200
projects / xen.git / log