Commit Graph
35651 Commits
Author SHA1 Message Date
Andrew Cooper 2ff229643b xen/livepatch: Don't crash on encountering STN_UNDEF relocations
A symndx of STN_UNDEF is special, and means a symbol value of 0.  While
legitimate in the ELF standard, its existance in a livepatch is questionable
at best.  Until a plausible usecase presents itself, reject such a relocation
with -EOPNOTSUPP.

Additionally, fix an off-by-one error while range checking symndx, and perform
a safety check on elf->sym[symndx].sym before derefencing it, to avoid
tripping over a NULL pointer when calculating val.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> [x86 and arm32]
Reviewed-by: Jan Beulich <JBeulich@suse.com>
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
2017-06-23 15:00:37 +01:00
Andrew Cooper bd53b85156 xen/livepatch: Use zeroed memory allocations for arrays
Each of these arrays is sparse.  Use zeroed allocations to cause uninitialised
array elements to contain deterministic values, most importantly for the
embedded pointers.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> [x86 and arm32]
Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
2017-06-23 15:00:37 +01:00
Andrew Cooper bf570f5935 xen/livepatch: Clean up arch relocation handling
* Reduce symbol scope and initalisation as much as possible
 * Annotate a fallthrough case in arm64
 * Fix switch statement style in arm32

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> [x86 and arm32]
2017-06-23 15:00:37 +01:00
Andrew Cooper d5bd9fe27e xen: Replace ASSERT(0) with ASSERT_UNREACHABLE()
No functional change, but the result is more informative both in the code and
error messages if the assertions do get hit.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Julien Grall <julien.gralL@arm.com>
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
2017-06-23 15:00:37 +01:00
Andrew Cooper 28022c62de x86/mm: Drop is_guest_l1_slot()
No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2017-06-23 15:00:37 +01:00
Andrew Cooper 2b8eb37993 x86/mm: Misc nonfunctional cleanup
* Drop trailing whitespace
 * Apply Xen comment and space style
 * Switch bool_t to bool
 * Drop TOGGLE_MODE() macro
 * Replace erroneous mandatory barriers with smp barriers
 * Switch boolean ints for real bools

No (intended) functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2017-06-23 15:00:37 +01:00
Razvan Cojocaru b20f8cd11f x86/monitor: Fixed CID 1412966: Memory - corruptions (OVERRUN)
Fixed an issue where the maximum index allowed (31) goes beyond the
actual number of array elements (4) of ad->monitor.write_ctrlreg_mask.
Coverity-ID: 1412966

Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2017-06-23 15:00:37 +01:00
Jan Beulich a15516c0cf make steal_page() return a proper error value
... and use it where suitable (the tmem caller doesn't propagate an
error code). While it doesn't matter as much, also make donate_page()
follow suit on x86 (on ARM it already returns -ENOSYS).

Also move their declarations to common code and add __must_check.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Julien Grall <julien.grall@arm.com>
2017-06-23 15:49:41 +02:00
Jan Beulich 4e59f578cb x86emul: simplify SHLD/SHRD handling
First of all there's no point considering the "shift == width" case,
when immediately before that check we mask "shift" by "width - 1". And
then truncate_word() use can be reduced too: dst.val, as obtained by
generic operand fetching code, is already suitably truncated, and its
use can also be made symmetric in the main conditional expression (on
only left shift results). Finally masking the result of a right shift
is not necessary when the left hand operand doesn't have more than
"width" significant bits.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2017-06-23 15:48:28 +02:00
Bernhard M. Wiedemann 579d698da6 etherboot: use gzip -n
to not include current timestamp in results
to allow for reproducible builds.

See https://reproducible-builds.org/ for why this matters

Signed-off-by: Bernhard M. Wiedemann <bwiedemann@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
2017-06-22 16:44:15 +01:00
Jan Beulich 4514f788d1 x86/mm: drop redundant domain parameter from get_page_from_gfn_p2m()
It can always be read from the passed p2m. Take the opportunity and
also rename the function, making the "p2m" suffix a prefix, to match
other p2m functions, and convert the "gfn" parameter's type.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
2017-06-22 09:58:07 +02:00
Jan Beulich 96b08455dc x86/mm: consolidate setting of TLB flush time stamps
Move code and comment into a helper function instead of repeating it in
multiple places.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2017-06-22 09:57:15 +02:00
Jan Beulich 3724385e39 x86/mmuext: don't allow copying/clearing non-RAM pages
The two operations really aren't meant for anything else.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2017-06-22 09:56:20 +02:00
Jan Beulich 31777889db gnttab: limit mapkind()'s iteration count
There's no need for the function to observe increases of the maptrack
table (which can occur as the maptrack lock isn't being held) - actual
population of maptrack entries is excluded while we're here (by way of
holding the respective grant table lock for writing, while code
populating entries acquires it for reading). Latch the limit ahead of
the loop, allowing for the barrier to move out, too.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2017-06-22 09:55:08 +02:00
George Dunlap 95906bf0f0 gnttab: remove host map in the event of a grant_map failure
The current code appropriately removes the reference and type counts
on failure, but leaves the mapping set up. As the only path which can
trigger this is failure from IOMMU manipulation, and as unprivileged
domains are being crashed in that case, this is not by itself a
security issue.

Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2017-06-22 09:53:18 +02:00
Jan Beulich 4149c9af7c x86: fold identical error paths in xenmem_add_to_physmap_one()
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2017-06-22 09:52:32 +02:00
Jan Beulich 0001015bbb ARM: simplify page type handling
There's no need to have anything here on ARM other than the distinction
between writable and non-writable pages (and even that could likely be
eliminated, but with a more intrusive change). Limit type to a single
bit and drop pinned and validated flags altogether.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
2017-06-22 09:51:29 +02:00
Jan Beulich c02edc7f28 evtchn: convert evtchn_port_is_*() to plain bool
... at once reducing overall source size by combining some statements
and constifying a few pointers.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2017-06-22 09:50:47 +02:00
Jan Beulich e42c68e8b5 domctl: restrict DOMCTL_set_target to HVM domains
Both the XSA-217 fix and
lists.xenproject.org/archives/html/xen-devel/2017-04/msg02945.html
make this assumption, so let's enforce it.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper@citrix.com>
2017-06-22 09:50:00 +02:00
Jan Beulich 5ec3ef77d0 gnttab: remove redundant xenheap check from gnttab_transfer()
The message isn't very useful, and the check is being done by
steal_page() anyway.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2017-06-22 09:49:03 +02:00
Jan Beulich c08a11ab98 public: adjust documentation following XSA-217
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2017-06-22 09:47:08 +02:00
Dario Faggioli 4463e7b980 idle_loop: either deal with tasklets or go idle
In fact, there are two kinds of tasklets: vCPU and
softirq context. When we want to do vCPU context tasklet
work, we force the idle vCPU (of a particular pCPU) into
execution, and run it from there.

This means there are two possible reasons for choosing
to run the idle vCPU:
1) we want a pCPU to go idle,
2) we want to run some vCPU context tasklet work.

If we're in case 2), it does not make sense to even
try to go idle (as the check will _always_ fail).

This patch rearranges the code of the body of idle
vCPUs, so that we actually check whether we are in
case 1) or 2), and act accordingly.

As a matter of fact, this also means that we do not
check if there's any tasklet work to do after waking
up from idle. This is not a problem, because:
a) for softirq context tasklets, if any is queued
   "during" wakeup from idle, TASKLET_SOFTIRQ is
   raised, and the call to do_softirq() (which is still
   happening *after* the wakeup) will take care of it;
b) for vCPU context tasklets, if any is queued "during"
   wakeup from idle, SCHEDULE_SOFTIRQ is raised and
   do_softirq() (happening after the wakeup) calls
   the scheduler. The scheduler sees that there is
   tasklet work pending and confirms the idle vCPU
   in execution, which then will get to execute
   do_tasklet().

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
2017-06-22 09:45:37 +02:00
Stefano Stabellini ded7303f16 docs: improve ARM passthrough doc
Add a warning: use passthrough with care.

Add a pointer to the gic device tree bindings. Add an explanation on how
to calculate irq numbers from device tree.

Add a brief explanation of the reg property and a pointer to the xl docs
for a description of the iomem property. Add a note that in the example
we are using different memory addresses for guests and host.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Acked-by: Julien Grall <julien.grall@arm.com>
2017-06-21 11:01:08 -07:00
Wei Liu 93de762bca x86/traps: remove now unused inclusion of emulate.h
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2017-06-21 14:30:24 +01:00
Wei Liu bb705e608d x86: move PV invalid op emulation code
Move the code to pv/emul-inv-op.c. Both functions are unchanged.
Provide pv_emulate_invalid_op and use it in traps.c.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2017-06-21 14:25:36 +01:00
Wei Liu d3dfd92e59 x86: move PV gate op emulation code
Move the code to pv/emul-gate-op.c. Prefix emulate_gate_op with pv_
and export it via pv/traps.h.

Pure code motion except for the rename.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2017-06-21 14:23:23 +01:00
Wei Liu ecaba067e1 x86: move PV privileged instruction emulation code
Move the code to pv/emul-priv-op.c. Prefix emulate_privileged_op with
pv_ and export it via pv/traps.h.

Also move gpr_switch.S since it is used by the privileged instruction
emulation code only.

Code motion only except for the rename. Cleanup etc will come later.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2017-06-21 14:23:23 +01:00
Wei Liu 46b488cb66 x86: factor out common PV emulation code
We're going to split PV emulation code into several files. This patch
extracts the functions needed by them into a dedicated file.

The functions are now prefixed with "pv_emul_" and exported via a
local header file.

While at it, change bool_t to bool.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2017-06-21 14:23:22 +01:00
Juergen Gross 48d0c82264 tools/xen-detect: try sysfs node for obtaining guest type
Instead of relying on cpuid instruction behaviour to tell which domain
type we are just try asking the kernel via the appropriate sysfs node
(added in Linux kernel 4.13).

Keep the old detection logic as a fallback for older kernels.

Signed-off-by: Juergen Gross <jgross@suse.com>
2017-06-21 14:15:55 +01:00
Petre Pircalabu d8f1b48fd6 xen-access: write_ctrlreg_c4 test
Add test for write_ctrlreg event handling.

Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
2017-06-20 17:13:56 +02:00
Petre Pircalabu 59aad28cfa x86/monitor: add masking support for write_ctrlreg events
Add support for filtering out the write_ctrlreg monitor events if they
are generated only by changing certains bits.
A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
function in order to mask the event generation if the changed bits are
set.

Signed-off-by: Petre Pircalabu <ppircalabu@bitdefender.com>
Acked-by: Tamas K Lengyel <tamas@tklengyel.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
2017-06-20 17:13:20 +02:00
Ross Lagerwall 4c09689153 rombios/ata: wait for BSY to clear after write
After rombios transfers the data for a write, it checks the status and
fails if BSY is set. qemu-trad doesn't set BSY for PIO writes, but QEMU
upstream does, and this causes rombios to fail writes because they are
marked as BSY. Instead, wait for BSY to clear after a write.

INT 13 writes are probably rarely used these days, but they are used by
GRUB 2 to write to its environment file which happens by default on
Ubuntu.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2017-06-20 17:13:02 +02:00
Ian Jackson 592e834522 xen/test/Makefile: Fix clean target, broken by pattern rule
In "xen/test/livepatch: Regularise Makefiles" we reworked
xen/test/Makefile to use a pattern rule.  However, there are two
problems with this.  Both are related to the way that xen/Rules.mk is
implicitly part of this Makefile because of the way that Makefiles
under xen/ are invoked by their parent directory Makefiles.

Firstly, the Rules.mk `clean' target overrides the pattern rule in
xen/test/Makefile.  The result is that `make -C xen clean' does not
actually run the livepatch clean target.

The Rules.mk clean target does have provision for recursing into
subdirectories, but that feature is tangled up with complex object
file iteration machinery which is not desirable here.  However, we can
extend the Rules.mk clean target since it is a double-colon rule.

Sadly this involves duplicating the SUBDIR iteration boilerplate.  (A
make function could be used but the cure would be worse than the
disease.)

Secondly, Rules.mk has a number of -include directives.  make likes to
try to (re)build files mentioned in includes.  With the % pattern
rule, this applies to those files too.

As a result, make -C xen clean would try to build `.*.d' (for example)
in xen/test.  This would fail with an error message.  The error would
be ignored because of the `-', but it's annoying and ugly.

Solve this by limiting the % pattern rule to the targets we expect it
to handle.  These are those listed in the top-level Makefile help
message, apart from: those which are subdir- or component-qualified;
clean targets (which are handled specially, even distclean); and dist,
src-tarball-*, etc. (which are converted to install by an earlier
Makefile).

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Tested-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Release-acked-by: Julien Grall <julien.grall@arm.com>
2017-06-20 15:11:42 +01:00
Jan Beulich b964e3106d memory: don't suppress P2M update in populate_physmap()
Commit d18627583d ("memory: don't hand MFN info to translated guests")
wrongly added a null-handle check there - just like stated in its
description for memory_exchange(), the array is also an input for
populate_physmap() (and hence can't reasonably be null). I have no idea
how I've managed to overlook this.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
2017-06-20 14:51:53 +02:00
Wolfram Strepp e95def2cb9 rb_tree: make clear distinction between two different cases in rb_erase()
There are two cases when a node, having 2 childs, is erased:
'normal case': the successor is not the right-hand-child of the node to be
erased
'special case': the successor is the right-hand child of the node to be erased

Here some ascii-art, with following symbols (referring to the code):
O: node to be deleted
N: the successor of O
P: parent of N
C: child of N
L: some other node

normal case:

                   O                         N
                  / \                       / \
                 /   \                     /   \
                L     \                   L     \
               / \     P      ---->      / \     P
                      / \                       / \
                     /                         /
                    N                         C
                     \                       / \
                      \
                       C
                      / \

special case:
                  O|P                        N
                  / \                       / \
                 /   \                     /   \
                L     \                   L     \
               / \     N      ---->      /       C
                        \                       / \
                         \
                          C
                         / \

Notice that for the special case we don't have to reconnect C to N.

Signed-off-by: Wolfram Strepp <wstrepp@gmx.de>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[Linux commit 4c60117811171d867d4f27f17ea07d7419d45dae]

Ported to Xen.

Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2017-06-20 14:51:07 +02:00
Wolfram Strepp 9e4f9e0649 rb_tree: reorganize code in rb_erase() for additional changes
First, move some code around in order to make the next change more obvious.

[akpm@linux-foundation.org: coding-style fixes]
Signed-off-by: Wolfram Strepp <wstrepp@gmx.de>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[Linux commit 16c047add3ceaf0ab882e3e094d1ec904d02312d]

Ported to Xen.

Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2017-06-20 14:50:39 +02:00
Wolfram Strepp e4b6d67ccd rbtree: optimize rb_erase()
Tfour 4 redundant if-conditions in function __rb_erase_color() in
lib/rbtree.c are removed.

In pseudo-source-code, the structure of the code is as follows:

if ((!A || B) && (!C || D)) {
.
.
.
} else {
		if (!C || D) {//if this is true, it implies: (A == true) && (B == false)
				if (A) {//hence this always evaluates to 'true'...
						.
				}
				.
				//at this point, C always becomes true, because of:
				__rb_rotate_right/left();
				//and:
				other = parent->rb_right/left;
		}
		.
		.
		if (C) {//...and this too !
				.
		}
}

Signed-off-by: Wolfram Strepp <wstrepp@gmx.de>
Acked-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[Linux commit 55a63998b8967615a15e2211ba0ff3a84a565824]

Ported to Xen.

Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2017-06-20 14:50:13 +02:00
Artem Bityutskiy b2f178fb35 rbtree: add const qualifier to some functions
The 'rb_first()', 'rb_last()', 'rb_next()' and 'rb_prev()' calls
take a pointer to an RB node or RB root. They do not change the
pointed objects, so add a 'const' qualifier in order to make life
of the users of these functions easier.

Indeed, if I have my own constant pointer &const struct my_type *p,
and I call 'rb_next(&p->rb)', I get a GCC warning:

warning: passing argument 1 of ‘rb_next’ discards qualifiers from pointer target
type

Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
[Linux commit f4b477c47332367d35686bd2b808c2156b96d7c7]

Ported to Xen.

Signed-off-by: Praveen Kumar <kpraveen.lkml@gmail.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
2017-06-20 14:49:40 +02:00
Roger Pau Monné 9b62c90308 x86/dpci: make sure hvm_do_IRQ_dpci is only called for HVM guests
While there add an ASSERT to hvm_do_IRQ_dpci.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
2017-06-20 14:49:11 +02:00
Julien Grall 725039d39e mm: don't use _{g,m}fn for defining INVALID_{G,M}FN
INVALID_{G,M}FN are defined using static inline helpers _{g,m}fn.
This means, they cannot be used to initialize a build time static variable:

In file included from mm.c:24:0:
xen/xen/include/xen/mm.h:59:26: error: initializer element is not constant
 #define INVALID_MFN      _mfn(~0UL)

Signed-off-by: Julien Grall <julien.grall@arm.com>
Acked-by: Tim Deegan <tim@xen.org>
[jb: add parentheses and const]
Acked-by: Jan Beulich <jbeulich@suse.com>
2017-06-20 14:48:10 +02:00
Julien Grall 6fb9419673 xen/arm: vgic: Sanitize target mask used to send SGI
The current function vgic_to_sgi does not sanitize the target mask and
may therefore get an invalid vCPU ID. This will result to an out of
bound access of d->vcpu[...] as there is no check whether the vCPU ID is
within the maximum supported by the guest.

This was introduced by commit ea37fd2111 "xen/arm: split vgic driver
into generic and vgic-v2 driver".

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
2017-06-20 14:47:07 +02:00
Jan Beulich 11fc7ccb72 gnttab: __gnttab_unmap_common_complete() is all-or-nothing
All failures have to be detected in __gnttab_unmap_common(), the
completion function must not skip part of its processing. In particular
the GNTMAP_device_map related putting of page references and adjustment
of pin count must not occur if __gnttab_unmap_common() signaled an
error. Furthermore the function must not make adjustments to global
state (here: clearing GNTTAB_device_map) before all possibly failing
operations have been performed.

There's one exception for IOMMU related failures: As IOMMU manipulation
occurs after GNTMAP_*_map have been cleared already, the related page
reference and pin count adjustments need to be done nevertheless. A
fundamental requirement for the correctness of this is that
iommu_{,un}map_page() crash any affected DomU in case of failure.

The version check appears to be pointless (or could perhaps be a
BUG_ON() or ASSERT()), but for the moment also move it.

This is part of XSA-224.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
2017-06-20 14:46:47 +02:00
George Dunlap 75b384ece6 gnttab: correct logic to get page references during map requests
The rules for reference counting are somewhat complicated:

* Each of GNTTAB_host_map and GNTTAB_device_map need their own
reference count

* If the mapping is writeable:
 - GNTTAB_host_map needs a type count under only some conditions
 - GNTTAB_device_map always needs a type count

If the mapping succeeds, we need to keep all of these; if the mapping
fails, we need to release whatever references we have acquired so far.

Additionally, the code that does a lot of this calculation "inherits"
a reference as part of the process of finding out who the owner is.

Finally, if the grant is mapped as writeable (without the
GNTMAP_readonly flag), but the hypervisor cannot grab a
PGT_writeable_page type, the entire operation should fail.

Unfortunately, the current code has several logic holes:

* If a grant is mapped only GNTTAB_device_map, and with a writeable
  mapping, but in conditions where a *host* type count is not
  necessary, the code will fail to grab the necessary type count.

* If a grant is mapped both GNTTAB_device_map and GNTTAB_host_map,
  with a writeable mapping, in conditions where the host type count is
  not necessary, *and* where the page cannot be changed to type
  PGT_writeable, the condition will not be detected.

In both cases, this means that on success, the type count will be
erroneously reduced when the grant is unmapped.  In the second case,
the type count will be erroneously reduced on the failure path as
well.  (In the first case the failure path logic has the same hole
as the reference grabbing logic.)

Additionally, the return value of get_page() is not checked; but this
may fail even if the first get_page() succeeded due to a reference
counting overflow.

First of all, simplify the restoration logic by explicitly counting
the reference and type references acquired.

Consider each mapping type separately, explicitly marking the
'incoming' reference as used so we know when we need to grab a second
one.

Finally, always check the return value of get_page[_type]() and go to
the failure path if appropriate.

This is part of XSA-224.

Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
2017-06-20 14:46:21 +02:00
Jan Beulich 56f2ab5b97 gnttab: never create host mapping unless asked to
We shouldn't create a host mapping unless asked to even in the case of
mapping a granted MMIO page. In particular the mapping wouldn't be torn
down when processing the matching unmap request.

This is part of XSA-224.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
2017-06-20 14:46:01 +02:00
George Dunlap 8fdfcb2b6b gnttab: fix handling of dev_bus_addr during unmap
If a grant has been mapped with the GNTTAB_device_map flag, calling
grant_unmap_ref() with dev_bus_addr set to zero should cause the
GNTTAB_device_map part of the mapping to be left alone.

Unfortunately, at the moment, op->dev_bus_addr is implicitly checked
before clearing the map and adjusting the pin count, but only the bits
above 12; and it is not checked at all before dropping page
references.  This means a guest can repeatedly make such a call to
cause the reference count to drop to zero, causing the page to be
freed and re-used, even though it's still mapped in its pagetables.

To fix this, always check op->dev_bus_addr explicitly for being
non-zero, as well as op->flag & GNTMAP_device_map, before doing
operations on the device_map.

While we're here, make the logic a bit cleaner:

* Always initialize op->frame to zero and set it from act->frame, to reduce the
chance of untrusted input being used

* Explicitly check the full dev_bus_addr against act->frame <<
  PAGE_SHIFT, rather than ignoring the lower 12 bits

This is part of XSA-224.

Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
2017-06-20 14:45:33 +02:00
Julien Grall c84e4b2dd4 arm: vgic: Don't update the LR when the IRQ is not enabled
gic_raise_inflight_irq will be called if the IRQ is already inflight
(i.e the IRQ is injected to the guest). If the IRQ is already already in
the LRs, then the associated LR will be updated.

To know if the interrupt is already in the LR, the function check if the
interrupt is queued. However, if the interrupt is not enabled then the
interrupt may not be queued nor in the LR. So gic_update_one_lr may be
called (if we inject on the current vCPU) and read the LR.

Because the interrupt is not in the LR, Xen will either read:
    * LR 0 if the interrupt was never injected before
    * LR 255 (GIC_INVALID_LR) if the interrupt was injected once. This
    is because gic_update_one_lr will reset p->lr.

Reading LR 0 will result to potentially update the wrong interrupt and
not keep the LRs in sync with Xen.

Reading LR 255 will result to:
    * Crash Xen on GICv3 as the LR index is bigger than supported (see
    gicv3_ich_read_lr).
    * Read/write always GICH_LR + 255 * 4 that is not part of the memory
    mapped.

The problem can be prevented by checking whether the interrupt is
enabled in gic_raise_inflight_irq before calling gic_update_one_lr.

A follow-up of this patch is expected to mitigate the issue in the
future.

This is XSA-223.

Signed-off-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
2017-06-20 14:41:55 +02:00
Jan Beulich a0cce6048d guest_physmap_remove_page() needs its return value checked
Callers, namely such subsequently freeing the page, must not blindly
assume success - the function may namely fail when needing to shatter a
super page, but there not being memory available for the then needed
intermediate page table.

As it happens, guest_remove_page() callers now also all check the
return value.

Furthermore a missed put_gfn() on an error path in gnttab_transfer() is
also being taken care of.

This is part of XSA-222.

Reported-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
2017-06-20 14:41:16 +02:00
Andrew Cooper b614f642c3 memory: fix return value handing of guest_remove_page()
Despite the description in mm.h, guest_remove_page() previously returned 0 for
paging errors.

Switch guest_remove_page() to having regular 0/-error semantics, and propagate
the return values from clear_mmio_p2m_entry() and mem_sharing_unshare_page()
to the callers (although decrease_reservation() is the only caller which
currently cares).

This is part of XSA-222.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
2017-06-20 14:39:56 +02:00
Jan Beulich e7719a0dfa evtchn: avoid NULL derefs
Commit fbbd5009e6 ("evtchn: refactor low-level event channel port ops")
added a de-reference of the struct evtchn pointer for a port without
first making sure the bucket pointer is non-NULL. This de-reference is
actually entirely unnecessary, as all relevant callers (beyond the
problematic do_poll()) already hold the port number in their hands, and
the actual leaf functions need nothing else.

For FIFO event channels there's a second problem in that the ordering
of reads and updates to ->num_evtchns and ->event_array[] was so far
undefined (the read side isn't always holding the domain's event lock).
Add respective barriers.

This is XSA-221.

Reported-by: Ankur Arora <ankur.a.arora@oracle.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
2017-06-20 14:37:47 +02:00
Jan Beulich de20bb6c4f x86: avoid leaking PKRU and BND* between vCPU-s
PKRU is explicitly "XSAVE-managed but not XSAVE-enabled", so guests
might access the register (via {RD,WR}PKRU) without setting XCR0.PKRU.
Force context switching as well as migrating the register as soon as
CR4.PKE is being set the first time.

For MPX (BND<n>, BNDCFGU, and BNDSTATUS) the situation is less clear,
and the SDM has not entirely consistent information for that case.
While experimentally the instructions don't change register state as
long as the two XCR0 bits aren't both 1, be on the safe side and enable
both if BNDCFGS.EN is being set the first time.

This is XSA-220.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
2017-06-20 14:36:51 +02:00