Ian Jackson [Fri, 14 Jun 2013 15:45:40 +0000 (16:45 +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.
* 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.
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.
Conflicts in Xen 4.1 version of the series:
* Trivial conflict due to elf_note_numeric_array not existing.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian Jackson [Fri, 14 Jun 2013 15:45:40 +0000 (16:45 +0100)]
libxc: Introduce xc_bitops.h
Copy the one file tools/libxc/xc_bitops.h from xen.git#aa1355f9.
We will need this for the next patch, which calls for a bitmap in
libxc.
xc_bitops.h was introduced to unify various existing sets of bitmap
operations. In this patch we backport only the introduction, not the
replacement of the other instances. So we introduce another instance
Sorry :-/.
This is part of the fix to a security issue, XSA-55.
This patch is unique to the Xen 4.1 version of the XSA-55 series.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian Jackson [Fri, 14 Jun 2013 15:45:39 +0000 (16:45 +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.
Conflicts in 4.1 series:
* xc_dom_load_elf_kernel has no rc variable to change.
* elf_load_image doesn't exist.
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:45:39 +0000 (16:45 +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>
Ian Jackson [Fri, 14 Jun 2013 15:45:39 +0000 (16:45 +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".
This is part of the fix to a security issue, XSA-55.
Differences in 4.1 backport:
* No xen/arch/arm.
* There was less error handling in xen/arch/x86/domain_build.c
so less need to change it.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian Jackson [Fri, 14 Jun 2013 15:45:38 +0000 (16:45 +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.
Conflicts in 4.1 backport:
* xen/arch/x86/bzimage.c in 4.1 doesn't use elf_is_elfbinary.
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:45:38 +0000 (16:45 +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.
Additional change in 4.1 backport:
* ELF_PRPTRVAL needs to be defined oddly on 4.1 and earlier because
Xen's headers provide no definitions of uintptr_t or PRIuPTR.
Conflicts:
* Callers of elf_load_binary don't check its return value in 4.1.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian Jackson [Fri, 14 Jun 2013 15:45:38 +0000 (16:45 +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:45:37 +0000 (16:45 +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.
Nontrivial differences in the 4.1 backport:
* We need to provide our own elf_uintptr_t since Xen doesn't.
* We see some additional differences in our verification diff.
* The "function-filter" needs to massage additional symbol names.
Conflicts:
* In xc_dom_load_elf_symtab the old code used
*(Elf64_Word*)(&shdr->e64.sh_name) and the new Elf32_Word
but in fact the type in the struct has changed too so the
new code using elf_store_field is still correct.
* loadelfimage, elf_load_image etc. don't exist and are done
directly with memcpy/memset; patch adjusted appropriately.
* elf_note_numeric_array doesn't exist in 4.1.
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.
* stubdom/grub/kexec.s2:
Large differences following ".section .debug_info" (which
the 4.1 build system erroneously fails to suppress).
* tools/libxc/xc_domain_restore.s2 (64-bit build):
One trivial code gen difference with no semantic import.
* 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__|_ctx|note_desc|types|last_order|memflags|mutex|d\d_cpu_last|write_count|wall_last|__PRETTY_FUNCTION__)\.(\d+)\:/ ||
m/^\s+\.local\s+(_ctx|write_count|d\d_cpu_last|wall_last|mutex)\.(\d+)\s*$/) {
$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:45:37 +0000 (16:45 +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:45:36 +0000 (16:45 +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>
v5: This patch is new in v5 of the series.
Ian Jackson [Fri, 14 Jun 2013 15:45:36 +0000 (16:45 +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:45:36 +0000 (16:45 +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>
Jan Beulich [Tue, 4 Jun 2013 07:41:41 +0000 (09:41 +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:40:12 +0000 (09:40 +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:38:49 +0000 (09:38 +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:28:18 +0000 (12:28 +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:27:52 +0000 (12:27 +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
Jan Beulich [Thu, 23 May 2013 13:19:38 +0000 (15:19 +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
Andrew Cooper [Thu, 23 May 2013 13:17:04 +0000 (15:17 +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 13:16:44 +0000 (15:16 +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
Jan Beulich [Thu, 23 May 2013 13:14:29 +0000 (15:14 +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.
Jan Beulich [Fri, 3 May 2013 07:46:54 +0000 (09:46 +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:27:13 +0000 (17:27 +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:26:43 +0000 (17:26 +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:26:13 +0000 (17:26 +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:22:36 +0000 (17:22 +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
Ian Jackson [Thu, 18 Apr 2013 16:44:17 +0000 (17:44 +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>
(cherry picked from commit 9f1a6ff38b8e7bb97a016794115de28553a6559f)
Conflicts:
tools/libxl/libxl.c Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Paul Durrant [Thu, 18 Apr 2013 15:38:17 +0000 (17:38 +0200)]
Fix rcu domain locking for transitive grants
When acquiring a transitive grant for copy then the owning domain
needs to be locked down as well as the granting domain. This was being
done, but the unlocking was not. The acquire code now stores the
struct domain * of the owning domain (rather than the domid) in the
active entry in the granting domain. The release code then does the
unlock on the owning domain. Note that I believe I also fixed a bug
where, for non-transitive grants the active entry contained a
reference to the acquiring domain rather than the granting
domain. From my reading of the code this would stop the release code
for transitive grants from terminating its recursion correctly.
Jan Beulich [Thu, 18 Apr 2013 14:24:08 +0000 (16:24 +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:23:07 +0000 (16:23 +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.
On 32-bit we also need to make sure we clear SYSENTER_CS for all CPUs
(neither #RESET nor #INIT guarantee this).
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.
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.
VMX: disable SMEP feature when guest is in non-paging mode
SMEP is disabled if CPU is in non-paging mode in hardware.
However Xen always uses paging mode to emulate guest non-paging
mode with HAP. To emulate this behavior, SMEP needs to be manually
disabled when guest switches to non-paging mode.
We met an issue that, SMP Linux guest with recent kernel (enable
SMEP support, for example, 3.5.3) would crash with triple fault if
setting unrestricted_guest=0 in grub. This is because Xen uses an
identity mapping page table to emulate the non-paging mode, where
the page table is set with USER flag. If SMEP is still enabled in
this case, guest will meet unhandlable page fault and then crash.
Tim Deegan [Tue, 9 Apr 2013 14:19:53 +0000 (16:19 +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
Ben Guthro [Tue, 9 Apr 2013 14:13:11 +0000 (16:13 +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
Jan Beulich [Tue, 9 Apr 2013 14:10:29 +0000 (16:10 +0200)]
x86: irq_move_cleanup_interrupt() must ignore legacy vectors
Since the main loop in the function includes legacy vectors, and since
vector_irq[] gets set up for legacy vectors regardless of whether those
get handled through the IO-APIC, it must not do anything on this vector
range. In fact, we should never get past the move_cleanup_count check
for IRQs not handled through the IO-APIC. Adding a respective assertion
woulkd make those iterations more expensive (due to the lock acquire).
For such an assertion to not have false positives we however ought to
suppress setting up IRQ2 as an 8259A interrupt (which wasn't correct
anyway), which is being done here despite the assertion not actually
getting added.
Furthermore, there's no point iterating over the vectors past
LAST_HIPRIORITY_VECTOR, so terminate the loop accordingly.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Keir Fraser <keir@xen.org>
master commit: af699220ad6d111ba76fc3040342184e423cc9a1
master date: 2013-04-02 08:30:03 +0200
To make erst table size checking code works on all systems, both
testing are treated as PASS.
Same situation applies to einj_tab->header_length, so corresponding
table size checking is changed in similar way too.
Originally-by: Yinghai Lu <yinghai@kernel.org> Signed-off-by: Huang Ying <ying.huang@intel.com>
- use switch() for better readability
- add comment explaining why a formally invalid size it also being
accepted
- check erst_tab->header.length before even looking at
erst_tab->header_length
- prefer sizeof(*erst_tab) over sizeof(struct acpi_table_erst)
Some actions in APEI ERST and EINJ tables are optional, for example,
ACPI_EINJ_BEGIN_OPERATION action is used to do some preparation for
error injection, and firmware may choose to do nothing here. While
some other actions are mandatory, for example, firmware must provide
ACPI_EINJ_GET_ERROR_TYPE implementation.
Original implementation treats all actions as optional (that is, can
have no instructions), that may cause issue if firmware does not
provide some mandatory actions. To fix this, this patch adds
apei_exec_run_optional, which should be used for optional actions.
The original apei_exec_run should be used for mandatory actions.
Signed-off-by: Huang Ying <ying.huang@intel.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 72af01bf6f7489e54ad59270222a29d3e8c501d1
master date: 2013-03-22 12:46:25 +0100
Andrew Cooper [Tue, 2 Apr 2013 10:37:28 +0000 (12:37 +0200)]
ACPI/APEI: Unlock apei_iomaps_lock on error path
This causes deadlocks during early boot on hardware with broken/buggy
APEI implementations, such as a Dell Poweredge 2950 with the latest
currently available BIOS.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Don't use goto or another special error path, as handling the error
case in normal flow is quite simple.
ACPI/APEI: fix ERST MOVE_DATA instruction implementation
The src_base and dst_base fields in apei_exec_context are physical
address, so they should be ioremaped before being used in ERST
MOVE_DATA instruction.
Reported-by: Javier Martinez Canillas <martinez.javier@gmail.com> Reported-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Huang Ying <ying.huang@intel.com>
Replace use of ioremap() by __acpi_map_table()/set_fixmap(). Fix error
handling.
x86: reserve pages when SandyBridge integrated graphics
SNB graphics devices have a bug that prevent them from accessing certain
memory ranges, namely anything below 1M and in the pages listed in the
table.
Xen does not initialize below 1MB to heap, i.e. below 1MB pages don't be
allocated, so it's unnecessary to reserve memory below the 1 MB mark
that has not already been reserved.
So reserve those pages listed in the table at xen boot if set detect a
SNB gfx device on the CPU to avoid GPU hangs.
Jan Beulich [Tue, 2 Apr 2013 10:31:36 +0000 (12:31 +0200)]
AMD IOMMU: allow disabling only interrupt remapping when certain IVRS consistency checks fail
After some more thought on the XSA-36 and specifically the comments we
got regarding disabling the IOMMU in this situation altogether making
things worse instead of better, I came to the conclusion that we can
actually restrict the action in affected cases to just disabling
interrupt remapping. That doesn't make the situation worse than prior
to the XSA-36 fixes (where interrupt remapping didn't really protect
domains from one another), but allows at least DMA isolation to still
be utilized.
To do so, disabling of interrupt remapping must be explicitly requested
on the command line - respective checks will then be skipped.
Stepping B-3 has two errata (#47 and #53) related to Interrupt
remapping, to which the workaround is for the BIOS to completely disable
interrupt remapping. These errata are fixed in stepping C-2.
Unfortunately this chipset stepping is very common and many BIOSes are
not disabling interrupt remapping on this stepping . We can detect this in
Xen and prevent Xen from using the problematic interrupt remapping feature.
The Intel 5500/5520/X58 chipset does not support VT-d
Extended Interrupt Mode(EIM). This means the iommu_supports_eim() check
always fails and so x2apic mode cannot be enabled in Xen before this quirk
disables the interrupt remapping feature.
Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Gate the function call to check the quirk on interrupt remapping being
requested to get enabled, and upon failure disable the IOMMU to be in
line with what the changes for XSA-36 (plus follow-ups) did.
Jan Beulich [Tue, 2 Apr 2013 10:29:34 +0000 (12:29 +0200)]
IOMMU: properly check whether interrupt remapping is enabled
... rather than the IOMMU as a whole.
That in turn required to make sure iommu_intremap gets properly
cleared when the respective initialization fails (or isn't being
done at all).
Along with making sure interrupt remapping doesn't get inconsistently
enabled on some IOMMUs and not on others in the VT-d code, this in turn
allowed quite a bit of cleanup on the VT-d side (removed from the
backport).
Andrew Cooper [Tue, 2 Apr 2013 10:27:20 +0000 (12:27 +0200)]
AMD/IOMMU: Process softirqs while building dom0 iommu mappings
Recent changes which have made their way into xen-4.2 stable have pushed the
runtime of construct_dom0() over 5 seconds, which has caused regressions in
XenServer testing because of our 5 second watchdog.
The root cause is that amd_iommu_dom0_init() does not process softirqs and in
particular the nmi_timer which causes the watchdog to decide that no useful
progress is being made.
This patch adds periodic calls to process_pending_softirqs() at the same
interval as the Intel variant of this function. The server which was failing
with the watchdog test now boots reliably with a timeout of 1 second.
Jan Beulich [Tue, 2 Apr 2013 10:26:22 +0000 (12:26 +0200)]
x86/MCA: suppress bank clearing for certain injected events
As the bits indicating validity of the ADDR and MISC bank MSRs may be
injected in a way that isn't consistent with what the underlying
hardware implements (while the bank must be valid for injection to
work, the auxiliary MSRs may not be implemented - and hence cause #GP
upon access - if the hardware never sets the corresponding valid bits.
Consequently we need to do the clearing writes only if no value was
interposed for the respective MSR (which also makes sense the other way
around: there's no point in clearing a hardware register when all data
read came from software). Of course this all requires the injection
tool to do things in a consistent way (but that had been a requirement
before already).
Signed-off-by: Jan Beulich <jbeulich@suse.com> Tested-by: Ren Yongjie <yongjie.ren@intel.com> Acked-by: Liu Jinsong <jinsong.liu@intel.com>
master changeset: b0583c0e64cc8bb6229c95c3304fdac2051f79b3
master date: 2013-03-12 15:53:30 +0100
Try to fix the the issue that "some AMD systems may round the
frequencies in ACPI tables to 100MHz boundaries. We can obtain the real
frequencies from MSRs, so add a quirk to fix these frequencies up
on AMD systems." (from f594065..)
In discussion (around 9855d8..) "it turned out that indeed real
HW/BIOSes may choose to not set the valid bit and thus mark the
P-state as invalid. So this could be considered a fix for broken
BIOSes." (from 9855d8..)
which is great for Linux. Unfortunatly the Linux kernel, when
it tries to do the RDMSR under Xen it fails to get the right
value (it gets zero) as Xen traps it and returns zero. Hence
when dom0 uploads the P-states they will be unmodified and
we should take care of updating the frequencies with the right
values.
I've tested it under Dell Inc. PowerEdge T105 /0RR825, BIOS 1.3.2
08/20/2008 where this quirk can be observed (x86 == 0x10, model == 2).
Also on other AMD (x86 == 0x12, A8-3850; x86 = 0x14, AMD E-350) to
make sure the quirk is not applied there.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Acked-by: stefan.bader@canonical.com
Do the MSR access here (and while at it, also the one reading
MSR_PSTATE_CUR_LIMIT) on the target CPU, and bound the loop over
amd_fixup_frequency() by max_hw_pstate (matching the one in
powernow_cpufreq_cpu_init()).
Ian Jackson [Fri, 15 Mar 2013 18:09:51 +0000 (18:09 +0000)]
Add DomU xz kernel decompression
Signed-off-by: Jan Beulich <jbeulich@novell.com>
xen-unstable changeset: 4afea3d65321c40bb8afec833c860f92176bfb42
xen-unstable date: Wed Mar 9 16:19:36 2011 +0000 Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
[ This is byte-for-byte identical to Bastian Blank's backport of the
same changeset to xen-4.1, as found in Debian xen_4.1.4-2.*
patch debian/patches/upstream-23002:eb64b8f8eebb -iwj ]
Jan Beulich [Tue, 12 Mar 2013 15:46:09 +0000 (16:46 +0100)]
x86/MSI: add mechanism to fully protect MSI-X table from PV guest accesses
This adds two new physdev operations for Dom0 to invoke when resource
allocation for devices is known to be complete, so that the hypervisor
can arrange for the respective MMIO ranges to be marked read-only
before an eventual guest getting such a device assigned even gets
started, such that it won't be able to set up writable mappings for
these MMIO ranges before Xen has a chance to protect them.
This also addresses another issue with the code being modified here,
in that so far write protection for the address ranges in question got
set up only once during the lifetime of a device (i.e. until either
system shutdown or device hot removal), while teardown happened when
the last interrupt was disposed of by the guest (which at least allowed
the tables to be writable when the device got assigned to a second
guest [instance] after the first terminated).
George Dunlap [Tue, 12 Mar 2013 15:30:41 +0000 (16:30 +0100)]
credit1: Use atomic bit operations for the flags structure
The flags structure is not protected by locks (or more precisely,
it is protected using an inconsistent set of locks); we therefore need
to make sure that all accesses are atomic-safe. This is particulary
important in the case of the PARKED flag, which if clobbered while
changing the YIELD bit will leave a vcpu wedged in an offline state.
Using the atomic bitops also requires us to change the size of the "flags"
element.
Spotted-by: Igor Pavlikevich <ipavlikevich@gmail.com> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
master changeset: be6507509454adf3bb5a50b9406c88504e996d5a
master date: 2013-03-04 13:37:39 +0100
Jan Beulich [Tue, 12 Mar 2013 15:30:04 +0000 (16:30 +0100)]
x86: defer processing events on the NMI exit path
Otherwise, we may end up in the scheduler, keeping NMIs masked for a
possibly unbounded period of time (until whenever the next IRET gets
executed). Enforce timely event processing by sending a self IPI.
Of course it's open for discussion whether to always use the straight
exit path from handle_ist_exception.
Jan Beulich [Tue, 12 Mar 2013 15:29:11 +0000 (16:29 +0100)]
SEDF: avoid gathering vCPU-s on pCPU0
The introduction of vcpu_force_reschedule() in 14320:215b799fa181 was
incompatible with the SEDF scheduler: Any vCPU using
VCPUOP_stop_periodic_timer (e.g. any vCPU of half way modern PV Linux
guests) ends up on pCPU0 after that call. Obviously, running all PV
guests' (and namely Dom0's) vCPU-s on pCPU0 causes problems for those
guests rather sooner than later.
So the main thing that was clearly wrong (and bogus from the beginning)
was the use of cpumask_first() in sedf_pick_cpu(). It is being replaced
by a construct that prefers to put back the vCPU on the pCPU that it
got launched on.
However, there's one more glitch: When reducing the affinity of a vCPU
temporarily, and then widening it again to a set that includes the pCPU
that the vCPU was last running on, the generic scheduler code would not
force a migration of that vCPU, and hence it would forever stay on the
pCPU it last ran on. Since that can again create a load imbalance, the
SEDF scheduler wants a migration to happen regardless of it being
apparently unnecessary.
Of course, an alternative to checking for SEDF explicitly in
vcpu_set_affinity() would be to introduce a flags field in struct
scheduler, and have SEDF set a "always-migrate-on-affinity-change"
flag.
Jan Beulich [Tue, 12 Mar 2013 15:24:58 +0000 (16:24 +0100)]
x86: make certain memory sub-ops return valid values
When a domain's shared info field "max_pfn" is zero,
domain_get_maximum_gpfn() so far returned ULONG_MAX, which
do_memory_op() in turn converted to -1 (i.e. -EPERM). Make the former
always return a sensible number (i.e. zero if the field was zero) and
have the latter no longer truncate return values.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Tim Deegan <tim@xen.org>
master changeset: 7ffc9779aa5120c5098d938cb88f69a1dda9a0fe
master date: 2013-03-04 10:16:04 +0100
Juergen Gross [Tue, 12 Mar 2013 15:23:15 +0000 (16:23 +0100)]
Avoid stale pointer when moving domain to another cpupool
When a domain is moved to another cpupool the scheduler private data pointers
in vcpu and domain structures must never point to an already freed memory
area.
While at it, simplify sched_init_vcpu() by using DOM2OP instead VCPU2OP.
xen, cpupools: Fix cpupool-move to make more consistent
The full order for creating new private data structures when moving
from one pool to another is now:
* Allocate all new structures
- Allocate a new private domain structure (but don't point there yet)
- Allocate per-vcpu data structures (but don't point there yet)
* Remove old structures
- Remove each vcpu, freeing the associated data structure
- Free the domain data structure
* Switch to the new structures
- Set the domain to the new cpupool, with the new private domain
structure
- Set each vcpu to the respective new structure, and insert
This is in line with a (fairly reasonable) assumption in credit2 that
the private structure of the domain will be the private structure
pointed to by the per-vcpu private structure.
Also fix a bug, in which insert_vcpu was called with the *old* vcpu
ops rather than the new ones.
Tim Deegan [Tue, 12 Mar 2013 15:22:03 +0000 (16:22 +0100)]
vmx: fix handling of NMI VMEXIT.
Call do_nmi() directly and explicitly re-enable NMIs rather than
raising an NMI through the APIC. Since NMIs are disabled after the
VMEXIT, the raised NMI would be blocked until the next IRET
instruction (i.e. the next real interrupt, or after scheduling a PV
guest) and in the meantime the guest will spin taking NMI VMEXITS.
Also, handle NMIs before re-enabling interrupts, since if we handle an
interrupt (and therefore IRET) before calling do_nmi(), we may end up
running the NMI handler with NMIs enabled.
Signed-off-by: Tim Deegan <tim@xen.org> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Acked-by: Jan Beulich <jbeulich@suse.com>
master changeset: 7dd3b06ff031c9a8c727df16c5def2afb382101c
master date: 2013-02-28 14:00:18 +0000
Jan Beulich [Fri, 8 Mar 2013 12:59:02 +0000 (13:59 +0100)]
x86: fix CMCI injection
This fixes the wrong use of literal vector 0xF7 with an "int"
instruction (invalidated by 25113:14609be41f36) and the fact that doing
the injection via a software interrupt was never valid anyway (because
cmci_interrupt() acks the LAPIC, which does the wrong thing if the
interrupt didn't get delivered though it).
In order to do latter, the patch introduces send_IPI_self(), at once
removing two opend coded uses of "genapic" in the IRQ handling code.
The IOMMU may stop processing page translations due to a perceived lack
of credits for writing upstream peripheral page service request (PPR)
or event logs. If the L2B miscellaneous clock gating feature is enabled
the IOMMU does not properly register credits after the log request has
completed, leading to a potential system hang.
BIOSes are supposed to disable L2B micellaneous clock gating by setting
L2_L2B_CK_GATE_CONTROL[CKGateL2BMiscDisable](D0F2xF4_x90[2]) = 1b. This
patch corrects that for those which do not enable this workaround.
Tim Deegan [Fri, 8 Mar 2013 12:46:22 +0000 (13:46 +0100)]
x86/setup: don't relocate the VGA hole.
Copying the contents of the VGA hole is at best pointless and at worst
dangerous. Booting Xen on Xen, it causes a very long delay as each
byte is referred to qemu.
Since we were already discarding the first 1MB of the relocated area,
just avoid copying it in the first place.
Reported-by: Jon Ludlam <jonathan.ludlam@eu.citrix.com> Signed-off-by: Tim Deegan <tim@xen.org>
master changeset: 0b76ce20de85ad7c23c47ee3275020859b91d46b
master date: 2013-02-14 12:20:58 +0000
Michael Young [Wed, 13 Feb 2013 17:00:15 +0000 (17:00 +0000)]
tools: Fix memset(&p,0,sizeof(p)) idiom in several places.
gcc 4.8 identifies several places where code of the form memset(x, 0,
sizeof(x)); is used incorrectly, meaning that less memory is set to
zero than required.
Signed-off-by: Michael Young <m.a.young@durham.ac.uk> Committed-by: Keir Fraser <keir@xen.org>
(cherry picked from commit d119301b5816b39b5ba722a2f8b301b37e8e34bd)
libxl: Fix uninitialized variable in libxl_create_stubdom
It is used for result domid from libxl__domain_make, but actually this
function have assert on an initial value.
This patch is intended for xen-4.1 only - 4.2 and later have reworked
this part of code already containing the fix.
Signed-off-by: Marek Marczykowski <marmarek@invisiblethingslab.com> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Dario Faggioli [Fri, 15 Feb 2013 14:31:55 +0000 (15:31 +0100)]
xen: sched_credit: improve picking up the idle CPU for a VCPU
In _csched_cpu_pick() we try to select the best possible CPU for
running a VCPU, considering the characteristics of the underlying
hardware (i.e., how many threads, core, sockets, and how busy they
are). What we want is "the idle execution vehicle with the most
idling neighbours in its grouping".
In order to achieve it, we select a CPU from the VCPU's affinity,
giving preference to its current processor if possible, as the basis
for the comparison with all the other CPUs. Problem is, to discount
the VCPU itself when computing this "idleness" (in an attempt to be
fair wrt its current processor), we arbitrarily and unconditionally
consider that selected CPU as idle, even when it is not the case,
for instance:
1. If the CPU is not the one where the VCPU is running (perhaps due
to the affinity being changed);
2. The CPU is where the VCPU is running, but it has other VCPUs in
its runq, so it won't go idle even if the VCPU in question goes.
22005(...) line (the first line) means _csched_cpu_pick() was called
on VCPU 1 of domain 10, while it is running on CPU 0, and it choose
CPU 8, which is busy ('|'), even if there are plenty of idle
CPUs. That is because, as a consequence of changing the VCPU affinity,
CPU 8 was chosen as the basis for the comparison, and therefore
considered idle (its bit gets unconditionally set in the bitmask
representing the idle CPUs). 28004(...) line means the VCPU is woken
up and queued on CPU 8's runq, where it waits for a context switch or
a migration, in order to be able to execute.
This change fixes things by only considering the "guessed" CPU idle if
the VCPU in question is both running there and is its only runnable
VCPU.
Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com> Acked-by: George Dunlap <george.dunlap@citrix.com>
xen-unstable changeset: 26287:127c2c47d440
xen-unstable date: Tue Dec 18 18:10:18 UTC 2012
Jan Beulich [Fri, 15 Feb 2013 14:30:38 +0000 (15:30 +0100)]
AMD IOMMU: also spot missing IO-APIC entries in IVRS table
Apart from dealing duplicate conflicting entries, we also have to
handle firmware omitting IO-APIC entries in IVRS altogether. Not doing
so has resulted in c/s 26517:601139e2b0db to crash such systems during
boot (whereas with the change here the IOMMU gets disabled just as is
being done in the other cases, i.e. unless global tables are being
used).
Debugging this issue has also pointed out that the debug log output is
pretty ugly to look at - consolidate the output, and add one extra
item for the IVHD special entries, so that future issues are easier
to analyze.
Signed-off-by: Jan Beulich <jbeulich@suse.com> Tested-by: Sander Eikelenboom <linux@eikelenboom.it> Acked-by: Ian Campbell <ian.campbell@citrix.com>
xen-unstable changeset: 26531:e68f14b9e739
xen-unstable date: Thu Feb 14 08:40:52 UTC 2013
Ian Campbell [Fri, 15 Feb 2013 11:50:45 +0000 (11:50 +0000)]
tools/ocaml: oxenstored: correctly handle a full ring.
Change 26521:2c0fd406f02c (part of XSA-38 / CVE-2013-0215) incorrectly
caused us to ignore rather than process a completely full ring. Check if
producer and consumer are equal before masking to avoid this, since prod ==
cons + PAGE_SIZE after masking becomes prod == cons.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com> Acked-by: Keir Fraser <keir@xen.org> Committed-by: Ian Campbell <ian.campbell@citrix.com>
xen-unstable changeset: 26539:759574df84a6 Backport-requested-by: security@xen.org Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Jan Beulich [Tue, 12 Feb 2013 12:33:19 +0000 (13:33 +0100)]
x86: restore (optional) forwarding of PCI SERR induced NMI to Dom0
c/s 22949:54fe1011f86b removed the forwarding of NMIs to Dom0 when they
were caused by PCI SERR. NMI buttons as well as BMCs (like HP's iLO)
may however want such events to be seen in Dom0 (e.g. to trigger a
dump).
Therefore restore most of the functionality which named c/s removed
(adjusted for subsequent changes, and adjusting the public interface to
use the modern term, retaining the old one for backwards
compatibility).
Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Acked-by: Keir Fraser <keir@xen.org>
xen-unstable changeset: 26440:5af4f2ab06f3
xen-unstable date: Tue Jan 22 08:33:10 UTC 2013
Boris Ostrovsky [Tue, 12 Feb 2013 12:32:05 +0000 (13:32 +0100)]
x86/AMD: Enable WC+ memory type on family 10 processors
In some cases BIOS may not enable WC+ memory type on family 10 processors,
instead converting what would be WC+ memory to CD type. On guests using
nested pages this could result in performance degradation. This patch
enables WC+.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com>
xen-unstable changeset: 26427:8f6dd5dc5d6c
xen-unstable date: Fri Jan 18 11:20:58 UTC 2013
Ian Jackson [Thu, 7 Feb 2013 14:26:37 +0000 (14:26 +0000)]
oxenstored: Enforce a maximum message size of 4096 bytes
The maximum size of a message is part of the protocol spec in
xen/include/public/io/xs_wire.h
Before this patch a client which sends an overly large message can
cause a buffer read overrun.
Note if a badly-behaved client sends a very large message
then it will be difficult for them to make their connection
work again-- they will probably need to reboot.
This is a security issue, part of XSA-38 / CVE-2013-0215.
Signed-off-by: David Scott <dave.scott@eu.citrix.com> Acked-by: Ian Campbell <Ian.Campbell@citrix.com> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
xen-unstable changeset: 26522:ffd30e7388ad Backport-requested-by: security@xen.org Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Ian Jackson [Thu, 7 Feb 2013 14:26:29 +0000 (14:26 +0000)]
tools/ocaml: oxenstored: Be more paranoid about ring reading
oxenstored makes use of the OCaml Xenbus bindings, in which the
function xs_ring_read in tools/ocaml/libs/xb/xs_ring_stubs.c is used
to read from the shared memory Xenstore ring.
This function does not correctly handle all possible (prod, cons)
states when MASK_XENSTORE_IDX(prod) > MASK_XENSTORE_IDX(cons).
The root cause is the use of the unmasked values of prod and cons to
calculate to_read. If prod is set to an out-of-range value, the ring
peer can cause to_read to be too large or even negative. This allows
the ring peer to force oxenstored to read and write out of range for
the buffers leading to a crash or possibly to privilege escalation.
Correct this by masking the values of cons and prod at the start, so
we only deal with masked values. This makes the logic simpler, as
semantically inappropriate values of the upper bits of the ring
pointers are simply ignored.
The same vulnerability does not exist in the ring writer because the
only use made of the unmasked value is the check which prevents the
prod pointer overtaking the cons pointer. A ring peer which defeats
this check will suffer only lost data.
However, additionally, precautions need to be taken to ensure that
req_cons and req_prod are only read once in each function. Without
the use of volatile or some asm construct, the compiler can "prove"
that req_cons and req_prod do not change unexpectedly and is permitted
to "amplify" the read of (say) req_cons into two reads at different
times, giving two different values for use as cons, and then use the
two sources of cons interchangeably. (The use of xen_mb() does not
forbid this.)
Therefore do the reads of req_cons and req_prod through a volatile
pointer in both xs_ring_read and xs_ring_write.
This is currently believed to be a theoretical vulnerability as we are
not aware of any compilers which amplify reads in this way.
This is a security issue, part of XSA-38 / CVE-2013-0215.
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com> Tested-by: Matthew Daley <mattjd@gmail.com> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
xen-unstable changeset: 26521:2c0fd406f02c Backport-requested-by: security@xen.org Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
Boris Ostrovsky [Tue, 5 Feb 2013 14:36:11 +0000 (15:36 +0100)]
AMD,IOMMU: Disable IOMMU if SATA Combined mode is on
AMD's SP5100 chipset can be placed into SATA Combined mode
that may cause prevent dom0 from booting when IOMMU is
enabled and per-device interrupt remapping table is used.
While SP5100 erratum 28 requires BIOSes to disable this mode,
some may still use it.
This patch checks whether this mode is on and, if per-device
table is in use, disables IOMMU.
This is XSA-36 / CVE-2013-0153.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com>
Flipped operands of && in amd_iommu_init() to make the message issued
by amd_sp5100_erratum28() match reality (when amd_iommu_perdev_intremap
is zero, there's really no point in calling the function).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
xen-unstable changeset: 26518:e379a23b0465
xen-unstable date: Tue Feb 5 14:21:25 UTC 2013
Jan Beulich [Tue, 5 Feb 2013 14:35:44 +0000 (15:35 +0100)]
AMD,IOMMU: Clean up old entries in remapping tables when creating new one
When changing the affinity of an IRQ associated with a passed
through PCI device, clear previous mapping.
This is XSA-36 / CVE-2013-0153.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
In addition, because some BIOSes may incorrectly program IVRS
entries for IOAPIC try to check for entry's consistency. Specifically,
if conflicting entries are found disable IOMMU if per-device
remapping table is used. If entries refer to bogus IOAPIC IDs
disable IOMMU unconditionally
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com>
xen-unstable changeset: 26517:601139e2b0db
xen-unstable date: Tue Feb 5 14:20:47 UTC 2013
Boris Ostrovsky [Tue, 5 Feb 2013 14:34:55 +0000 (15:34 +0100)]
ACPI: acpi_table_parse() should return handler's error code
Currently, the error code returned by acpi_table_parse()'s handler
is ignored. This patch will propagate handler's return value to
acpi_table_parse()'s caller.
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@amd.com> Committed-by: Jan Beulich <jbeulich@suse.com>
xen-unstable changeset: 26516:32d4516a97f0
xen-unstable date: Tue Feb 5 14:18:18 UTC 2013
projects / xen.git / log