system/xen: XSA 191-198 update.

Signed-off-by: Mario Preksavec <mario@slackware.hr>

Signed-off-by: Willy Sudiarto Raharjo <willysr@slackbuilds.org>
This commit is contained in:
Mario Preksavec 2016-11-27 18:20:26 +01:00 committed by Willy Sudiarto Raharjo
parent 35b0451cf4
commit 393ee9907c
No known key found for this signature in database
GPG key ID: 887B8374D7333381
11 changed files with 801 additions and 1 deletions

View file

@ -24,7 +24,7 @@
PRGNAM=xen PRGNAM=xen
VERSION=${VERSION:-4.7.1} VERSION=${VERSION:-4.7.1}
BUILD=${BUILD:-1} BUILD=${BUILD:-2}
TAG=${TAG:-_SBo} TAG=${TAG:-_SBo}
SEABIOS=${SEABIOS:-1.9.2} SEABIOS=${SEABIOS:-1.9.2}

152
system/xen/xsa/xsa191.patch Normal file
View file

@ -0,0 +1,152 @@
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86/hvm: Fix the handling of non-present segments
In 32bit, the data segments may be NULL to indicate that the segment is
ineligible for use. In both 32bit and 64bit, the LDT selector may be NULL to
indicate that the entire LDT is ineligible for use. However, nothing in Xen
actually checks for this condition when performing other segmentation
checks. (Note however that limit and writeability checks are correctly
performed).
Neither Intel nor AMD specify the exact behaviour of loading a NULL segment.
Experimentally, AMD zeroes all attributes but leaves the base and limit
unmodified. Intel zeroes the base, sets the limit to 0xfffffff and resets the
attributes to just .G and .D/B.
The use of the segment information in the VMCB/VMCS is equivalent to a native
pipeline interacting with the segment cache. The present bit can therefore
have a subtly different meaning, and it is now cooked to uniformly indicate
whether the segment is usable or not.
GDTR and IDTR don't have access rights like the other segments, but for
consistency, they are treated as being present so no special casing is needed
elsewhere in the segmentation logic.
AMD hardware does not consider the present bit for %cs and %tr, and will
function as if they were present. They are therefore unconditionally set to
present when reading information from the VMCB, to maintain the new meaning of
usability.
Intel hardware has a separate unusable bit in the VMCS segment attributes.
This bit is inverted and stored in the present field, so the hvm code can work
with architecturally-common state.
This is XSA-191.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
xen/arch/x86/hvm/hvm.c | 8 ++++++++
xen/arch/x86/hvm/svm/svm.c | 4 ++++
xen/arch/x86/hvm/vmx/vmx.c | 20 +++++++++++---------
xen/arch/x86/x86_emulate/x86_emulate.c | 4 ++++
4 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 704fd64..deb1783 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2512,6 +2512,10 @@ bool_t hvm_virtual_to_linear_addr(
*/
addr = (uint32_t)(addr + reg->base);
+ /* Segment not valid for use (cooked meaning of .p)? */
+ if ( !reg->attr.fields.p )
+ goto out;
+
switch ( access_type )
{
case hvm_access_read:
@@ -2767,6 +2771,10 @@ static int hvm_load_segment_selector(
hvm_get_segment_register(
v, (sel & 4) ? x86_seg_ldtr : x86_seg_gdtr, &desctab);
+ /* Segment not valid for use (cooked meaning of .p)? */
+ if ( !desctab.attr.fields.p )
+ goto fail;
+
/* Check against descriptor table limit. */
if ( ((sel & 0xfff8) + 7) > desctab.limit )
goto fail;
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 16427f6..4cba406 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -627,6 +627,7 @@ static void svm_get_segment_register(struct vcpu *v, enum x86_segment seg,
{
case x86_seg_cs:
memcpy(reg, &vmcb->cs, sizeof(*reg));
+ reg->attr.fields.p = 1;
reg->attr.fields.g = reg->limit > 0xFFFFF;
break;
case x86_seg_ds:
@@ -660,13 +661,16 @@ static void svm_get_segment_register(struct vcpu *v, enum x86_segment seg,
case x86_seg_tr:
svm_sync_vmcb(v);
memcpy(reg, &vmcb->tr, sizeof(*reg));
+ reg->attr.fields.p = 1;
reg->attr.fields.type |= 0x2;
break;
case x86_seg_gdtr:
memcpy(reg, &vmcb->gdtr, sizeof(*reg));
+ reg->attr.bytes = 0x80;
break;
case x86_seg_idtr:
memcpy(reg, &vmcb->idtr, sizeof(*reg));
+ reg->attr.bytes = 0x80;
break;
case x86_seg_ldtr:
svm_sync_vmcb(v);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 9a8f694..a652c52 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1035,10 +1035,12 @@ void vmx_get_segment_register(struct vcpu *v, enum x86_segment seg,
reg->sel = sel;
reg->limit = limit;
- reg->attr.bytes = (attr & 0xff) | ((attr >> 4) & 0xf00);
- /* Unusable flag is folded into Present flag. */
- if ( attr & (1u<<16) )
- reg->attr.fields.p = 0;
+ /*
+ * Fold VT-x representation into Xen's representation. The Present bit is
+ * unconditionally set to the inverse of unusable.
+ */
+ reg->attr.bytes =
+ (!(attr & (1u << 16)) << 7) | (attr & 0x7f) | ((attr >> 4) & 0xf00);
/* Adjust for virtual 8086 mode */
if ( v->arch.hvm_vmx.vmx_realmode && seg <= x86_seg_tr
@@ -1118,11 +1120,11 @@ static void vmx_set_segment_register(struct vcpu *v, enum x86_segment seg,
}
}
- attr = ((attr & 0xf00) << 4) | (attr & 0xff);
-
- /* Not-present must mean unusable. */
- if ( !reg->attr.fields.p )
- attr |= (1u << 16);
+ /*
+ * Unfold Xen representation into VT-x representation. The unusable bit
+ * is unconditionally set to the inverse of present.
+ */
+ attr = (!(attr & (1u << 7)) << 16) | ((attr & 0xf00) << 4) | (attr & 0xff);
/* VMX has strict consistency requirement for flag G. */
attr |= !!(limit >> 20) << 15;
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index 7a707dc..7cb6f98 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1367,6 +1367,10 @@ protmode_load_seg(
&desctab, ctxt)) )
return rc;
+ /* Segment not valid for use (cooked meaning of .p)? */
+ if ( !desctab.attr.fields.p )
+ goto raise_exn;
+
/* Check against descriptor table limit. */
if ( ((sel & 0xfff8) + 7) > desctab.limit )
goto raise_exn;

View file

@ -0,0 +1,64 @@
From: Jan Beulich <jbeulich@suse.com>
Subject: x86/HVM: don't load LDTR with VM86 mode attrs during task switch
Just like TR, LDTR is purely a protected mode facility and hence needs
to be loaded accordingly. Also move its loading to where it
architecurally belongs.
This is XSA-192.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2728,17 +2728,16 @@ static void hvm_unmap_entry(void *p)
}
static int hvm_load_segment_selector(
- enum x86_segment seg, uint16_t sel)
+ enum x86_segment seg, uint16_t sel, unsigned int eflags)
{
struct segment_register desctab, cs, segr;
struct desc_struct *pdesc, desc;
u8 dpl, rpl, cpl;
bool_t writable;
int fault_type = TRAP_invalid_tss;
- struct cpu_user_regs *regs = guest_cpu_user_regs();
struct vcpu *v = current;
- if ( regs->eflags & X86_EFLAGS_VM )
+ if ( eflags & X86_EFLAGS_VM )
{
segr.sel = sel;
segr.base = (uint32_t)sel << 4;
@@ -2986,6 +2985,8 @@ void hvm_task_switch(
if ( rc != HVMCOPY_okay )
goto out;
+ if ( hvm_load_segment_selector(x86_seg_ldtr, tss.ldt, 0) )
+ goto out;
if ( hvm_set_cr3(tss.cr3, 1) )
goto out;
@@ -3008,13 +3009,12 @@ void hvm_task_switch(
}
exn_raised = 0;
- if ( hvm_load_segment_selector(x86_seg_ldtr, tss.ldt) ||
- hvm_load_segment_selector(x86_seg_es, tss.es) ||
- hvm_load_segment_selector(x86_seg_cs, tss.cs) ||
- hvm_load_segment_selector(x86_seg_ss, tss.ss) ||
- hvm_load_segment_selector(x86_seg_ds, tss.ds) ||
- hvm_load_segment_selector(x86_seg_fs, tss.fs) ||
- hvm_load_segment_selector(x86_seg_gs, tss.gs) )
+ if ( hvm_load_segment_selector(x86_seg_es, tss.es, tss.eflags) ||
+ hvm_load_segment_selector(x86_seg_cs, tss.cs, tss.eflags) ||
+ hvm_load_segment_selector(x86_seg_ss, tss.ss, tss.eflags) ||
+ hvm_load_segment_selector(x86_seg_ds, tss.ds, tss.eflags) ||
+ hvm_load_segment_selector(x86_seg_fs, tss.fs, tss.eflags) ||
+ hvm_load_segment_selector(x86_seg_gs, tss.gs, tss.eflags) )
exn_raised = 1;
rc = hvm_copy_to_guest_virt(

View file

@ -0,0 +1,68 @@
From: Jan Beulich <jbeulich@suse.com>
Subject: x86/PV: writes of %fs and %gs base MSRs require canonical addresses
Commit c42494acb2 ("x86: fix FS/GS base handling when using the
fsgsbase feature") replaced the use of wrmsr_safe() on these paths
without recognizing that wr{f,g}sbase() use just wrmsrl() and that the
WR{F,G}SBASE instructions also raise #GP for non-canonical input.
Similarly arch_set_info_guest() needs to prevent non-canonical
addresses from getting stored into state later to be loaded by context
switch code. For consistency also check stack pointers and LDT base.
DR0..3, otoh, already get properly checked in set_debugreg() (albeit
we discard the error there).
The SHADOW_GS_BASE check isn't strictly necessary, but I think we
better avoid trying the WRMSR if we know it's going to fail.
This is XSA-193.
Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -890,7 +890,13 @@ int arch_set_info_guest(
{
if ( !compat )
{
- if ( !is_canonical_address(c.nat->user_regs.eip) ||
+ if ( !is_canonical_address(c.nat->user_regs.rip) ||
+ !is_canonical_address(c.nat->user_regs.rsp) ||
+ !is_canonical_address(c.nat->kernel_sp) ||
+ (c.nat->ldt_ents && !is_canonical_address(c.nat->ldt_base)) ||
+ !is_canonical_address(c.nat->fs_base) ||
+ !is_canonical_address(c.nat->gs_base_kernel) ||
+ !is_canonical_address(c.nat->gs_base_user) ||
!is_canonical_address(c.nat->event_callback_eip) ||
!is_canonical_address(c.nat->syscall_callback_eip) ||
!is_canonical_address(c.nat->failsafe_callback_eip) )
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2723,19 +2723,22 @@ static int emulate_privileged_op(struct
switch ( regs->_ecx )
{
case MSR_FS_BASE:
- if ( is_pv_32bit_domain(currd) )
+ if ( is_pv_32bit_domain(currd) ||
+ !is_canonical_address(msr_content) )
goto fail;
wrfsbase(msr_content);
v->arch.pv_vcpu.fs_base = msr_content;
break;
case MSR_GS_BASE:
- if ( is_pv_32bit_domain(currd) )
+ if ( is_pv_32bit_domain(currd) ||
+ !is_canonical_address(msr_content) )
goto fail;
wrgsbase(msr_content);
v->arch.pv_vcpu.gs_base_kernel = msr_content;
break;
case MSR_SHADOW_GS_BASE:
- if ( is_pv_32bit_domain(currd) )
+ if ( is_pv_32bit_domain(currd) ||
+ !is_canonical_address(msr_content) )
goto fail;
if ( wrmsr_safe(MSR_SHADOW_GS_BASE, msr_content) )
goto fail;

144
system/xen/xsa/xsa194.patch Normal file
View file

@ -0,0 +1,144 @@
From 71096b016f7fd54a72af73576948cb25cf42ebcb Mon Sep 17 00:00:00 2001
From: Roger Pau Monné <roger.pau@citrix.com>Date: Wed, 2 Nov 2016 15:02:00 +0000
Subject: [PATCH] libelf: fix stack memory leak when loading 32 bit symbol
tables
The 32 bit Elf structs are smaller than the 64 bit ones, which means that
when loading them there's some padding left uninitialized at the end of each
struct (because the size indicated in e_ehsize and e_shentsize is
smaller than the size of elf_ehdr and elf_shdr).
Fix this by introducing a new helper that is used to set
[caller_]xdest_{base/size} and that takes care of performing the appropriate
memset of the region. This newly introduced helper is then used to set and
unset xdest_{base/size} in elf_load_bsdsyms. Now that the full struct
is zeroed, there's no need to specifically zero the undefined section.
This is XSA-194.
Suggested-by: Ian Jackson <ian.jackson@eu.citrix.com>
Also remove the open coded (and redundant with the earlier
elf_memset_unchecked()) use of caller_xdest_* from elf_init().
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
---
xen/common/libelf/libelf-loader.c | 14 +++-----------
xen/common/libelf/libelf-tools.c | 11 +++++++++--
xen/include/xen/libelf.h | 15 +++++++++------
3 files changed, 21 insertions(+), 19 deletions(-)
diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c
index 4d3ae4d..bc1f87b 100644
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -43,8 +43,6 @@ elf_errorstatus elf_init(struct elf_binary *elf, const char *image_input, size_t
elf->ehdr = ELF_MAKE_HANDLE(elf_ehdr, (elf_ptrval)image_input);
elf->class = elf_uval_3264(elf, elf->ehdr, e32.e_ident[EI_CLASS]);
elf->data = elf_uval_3264(elf, elf->ehdr, e32.e_ident[EI_DATA]);
- elf->caller_xdest_base = NULL;
- elf->caller_xdest_size = 0;
/* Sanity check phdr. */
offset = elf_uval(elf, elf->ehdr, e_phoff) +
@@ -284,9 +282,8 @@ do { \
#define SYMTAB_INDEX 1
#define STRTAB_INDEX 2
- /* Allow elf_memcpy_safe to write to symbol_header. */
- elf->caller_xdest_base = &header;
- elf->caller_xdest_size = sizeof(header);
+ /* Allow elf_memcpy_safe to write to header. */
+ elf_set_xdest(elf, &header, sizeof(header));
/*
* Calculate the position of the various elements in GUEST MEMORY SPACE.
@@ -319,11 +316,7 @@ do { \
elf_store_field_bitness(elf, header_handle, e_phentsize, 0);
elf_store_field_bitness(elf, header_handle, e_phnum, 0);
- /* Zero the undefined section. */
- section_handle = ELF_MAKE_HANDLE(elf_shdr,
- ELF_REALPTR2PTRVAL(&header.elf_header.section[SHN_UNDEF]));
shdr_size = elf_uval(elf, elf->ehdr, e_shentsize);
- elf_memset_safe(elf, ELF_HANDLE_PTRVAL(section_handle), 0, shdr_size);
/*
* The symtab section header is going to reside in section[SYMTAB_INDEX],
@@ -404,8 +397,7 @@ do { \
}
/* Remove permissions from elf_memcpy_safe. */
- elf->caller_xdest_base = NULL;
- elf->caller_xdest_size = 0;
+ elf_set_xdest(elf, NULL, 0);
#undef SYMTAB_INDEX
#undef STRTAB_INDEX
diff --git a/xen/common/libelf/libelf-tools.c b/xen/common/libelf/libelf-tools.c
index 5a4757b..e73e729 100644
--- a/xen/common/libelf/libelf-tools.c
+++ b/xen/common/libelf/libelf-tools.c
@@ -59,8 +59,7 @@ bool elf_access_ok(struct elf_binary * elf,
return 1;
if ( elf_ptrval_in_range(ptrval, size, elf->dest_base, elf->dest_size) )
return 1;
- if ( elf_ptrval_in_range(ptrval, size,
- elf->caller_xdest_base, elf->caller_xdest_size) )
+ if ( elf_ptrval_in_range(ptrval, size, elf->xdest_base, elf->xdest_size) )
return 1;
elf_mark_broken(elf, "out of range access");
return 0;
@@ -373,6 +372,14 @@ bool elf_phdr_is_loadable(struct elf_binary *elf, ELF_HANDLE_DECL(elf_phdr) phdr
return ((p_type == PT_LOAD) && (p_flags & (PF_R | PF_W | PF_X)) != 0);
}
+void elf_set_xdest(struct elf_binary *elf, void *addr, uint64_t size)
+{
+ elf->xdest_base = addr;
+ elf->xdest_size = size;
+ if ( addr != NULL )
+ elf_memset_safe(elf, ELF_REALPTR2PTRVAL(addr), 0, size);
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h
index 95b5370..cf62bc7 100644
--- a/xen/include/xen/libelf.h
+++ b/xen/include/xen/libelf.h
@@ -210,13 +210,11 @@ struct elf_binary {
uint64_t bsd_symtab_pend;
/*
- * caller's other acceptable destination
- *
- * Again, these are trusted and must be valid (or 0) so long
- * as the struct elf_binary is in use.
+ * caller's other acceptable destination.
+ * Set by elf_set_xdest. Do not set these directly.
*/
- void *caller_xdest_base;
- uint64_t caller_xdest_size;
+ void *xdest_base;
+ uint64_t xdest_size;
#ifndef __XEN__
/* misc */
@@ -494,5 +492,10 @@ static inline void ELF_ADVANCE_DEST(struct elf_binary *elf, uint64_t amount)
}
}
+/* Specify a (single) additional destination, to which the image may
+ * cause writes. As with dest_base and dest_size, the values provided
+ * are trusted and must be valid so long as the struct elf_binary
+ * is in use or until elf_set_xdest(,0,0) is called. */
+void elf_set_xdest(struct elf_binary *elf, void *addr, uint64_t size);
#endif /* __XEN_LIBELF_H__ */
--
2.1.4

View file

@ -0,0 +1,45 @@
From: Jan Beulich <jbeulich@suse.com>
Subject: x86emul: fix huge bit offset handling
We must never chop off the high 32 bits.
This is XSA-195.
Reported-by: George Dunlap <george.dunlap@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -2549,6 +2549,12 @@ x86_emulate(
else
{
/*
+ * Instructions such as bt can reference an arbitrary offset from
+ * their memory operand, but the instruction doing the actual
+ * emulation needs the appropriate op_bytes read from memory.
+ * Adjust both the source register and memory operand to make an
+ * equivalent instruction.
+ *
* EA += BitOffset DIV op_bytes*8
* BitOffset = BitOffset MOD op_bytes*8
* DIV truncates towards negative infinity.
@@ -2560,14 +2566,15 @@ x86_emulate(
src.val = (int32_t)src.val;
if ( (long)src.val < 0 )
{
- unsigned long byte_offset;
- byte_offset = op_bytes + (((-src.val-1) >> 3) & ~(op_bytes-1));
+ unsigned long byte_offset =
+ op_bytes + (((-src.val - 1) >> 3) & ~(op_bytes - 1L));
+
ea.mem.off -= byte_offset;
src.val = (byte_offset << 3) + src.val;
}
else
{
- ea.mem.off += (src.val >> 3) & ~(op_bytes - 1);
+ ea.mem.off += (src.val >> 3) & ~(op_bytes - 1L);
src.val &= (op_bytes << 3) - 1;
}
}

View file

@ -0,0 +1,61 @@
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86/emul: Correct the IDT entry calculation in inject_swint()
The logic, as introduced in c/s 36ebf14ebe "x86/emulate: support for emulating
software event injection" is buggy. The size of an IDT entry depends on long
mode being active, not the width of the code segment currently in use.
In particular, this means that a compatibility code segment which hits
emulation for software event injection will end up using an incorrect offset
in the IDT for DPL/Presence checking. In practice, this only occurs on old
AMD hardware lacking NRip support; all newer AMD hardware, and all Intel
hardware bypass this path in the emulator.
While here, fix a minor issue with reading the IDT entry. The return value
from ops->read() wasn't checked, but in reality the only failure case is if a
pagefault occurs. This is not a realistic problem as the kernel will almost
certainly crash with a double fault if this setup actually occured.
This is part of XSA-196.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
xen/arch/x86/x86_emulate/x86_emulate.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index 7a707dc..f74aa8f 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1630,10 +1630,16 @@ static int inject_swint(enum x86_swint_type type,
{
if ( !in_realmode(ctxt, ops) )
{
- unsigned int idte_size = (ctxt->addr_size == 64) ? 16 : 8;
- unsigned int idte_offset = vector * idte_size;
+ unsigned int idte_size, idte_offset;
struct segment_register idtr;
uint32_t idte_ctl;
+ int lm = in_longmode(ctxt, ops);
+
+ if ( lm < 0 )
+ return X86EMUL_UNHANDLEABLE;
+
+ idte_size = lm ? 16 : 8;
+ idte_offset = vector * idte_size;
/* icebp sets the External Event bit despite being an instruction. */
error_code = (vector << 3) | ECODE_IDT |
@@ -1661,8 +1667,9 @@ static int inject_swint(enum x86_swint_type type,
* Should strictly speaking read all 8/16 bytes of an entry,
* but we currently only care about the dpl and present bits.
*/
- ops->read(x86_seg_none, idtr.base + idte_offset + 4,
- &idte_ctl, sizeof(idte_ctl), ctxt);
+ if ( (rc = ops->read(x86_seg_none, idtr.base + idte_offset + 4,
+ &idte_ctl, sizeof(idte_ctl), ctxt)) )
+ goto done;
/* Is this entry present? */
if ( !(idte_ctl & (1u << 15)) )

View file

@ -0,0 +1,76 @@
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: x86/svm: Fix injection of software interrupts
The non-NextRip logic in c/s 36ebf14eb "x86/emulate: support for emulating
software event injection" was based on an older version of the AMD software
manual. The manual was later corrected, following findings from that series.
I took the original wording of "not supported without NextRIP" to mean that
X86_EVENTTYPE_SW_INTERRUPT was not eligible for use. It turns out that this
is not the case, and the new wording is clearer on the matter.
Despite testing the original patch series on non-NRip hardware, the
swint-emulation XTF test case focuses on the debug vectors; it never ended up
executing an `int $n` instruction for a vector which wasn't also an exception.
During a vmentry, the use of X86_EVENTTYPE_HW_EXCEPTION comes with a vector
check to ensure that it is only used with exception vectors. Xen's use of
X86_EVENTTYPE_HW_EXCEPTION for `int $n` injection has always been buggy on AMD
hardware.
Fix this by always using X86_EVENTTYPE_SW_INTERRUPT.
Print and decode the eventinj information in svm_vmcb_dump(), as it has
several invalid combinations which cause vmentry failures.
This is part of XSA-196.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
xen/arch/x86/hvm/svm/svm.c | 13 +++++--------
xen/arch/x86/hvm/svm/svmdebug.c | 4 ++++
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 4391744..76efc3e 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1231,17 +1231,14 @@ static void svm_inject_trap(const struct hvm_trap *trap)
{
case X86_EVENTTYPE_SW_INTERRUPT: /* int $n */
/*
- * Injection type 4 (software interrupt) is only supported with
- * NextRIP support. Without NextRIP, the emulator will have performed
- * DPL and presence checks for us.
+ * Software interrupts (type 4) cannot be properly injected if the
+ * processor doesn't support NextRIP. Without NextRIP, the emulator
+ * will have performed DPL and presence checks for us, and will have
+ * moved eip forward if appropriate.
*/
if ( cpu_has_svm_nrips )
- {
vmcb->nextrip = regs->eip + _trap.insn_len;
- event.fields.type = X86_EVENTTYPE_SW_INTERRUPT;
- }
- else
- event.fields.type = X86_EVENTTYPE_HW_EXCEPTION;
+ event.fields.type = X86_EVENTTYPE_SW_INTERRUPT;
break;
case X86_EVENTTYPE_PRI_SW_EXCEPTION: /* icebp */
diff --git a/xen/arch/x86/hvm/svm/svmdebug.c b/xen/arch/x86/hvm/svm/svmdebug.c
index ded5d19..f93dfed 100644
--- a/xen/arch/x86/hvm/svm/svmdebug.c
+++ b/xen/arch/x86/hvm/svm/svmdebug.c
@@ -48,6 +48,10 @@ void svm_vmcb_dump(const char *from, struct vmcb_struct *vmcb)
vmcb->tlb_control,
(unsigned long long)vmcb->_vintr.bytes,
(unsigned long long)vmcb->interrupt_shadow);
+ printk("eventinj %016"PRIx64", valid? %d, ec? %d, type %u, vector %#x\n",
+ vmcb->eventinj.bytes, vmcb->eventinj.fields.v,
+ vmcb->eventinj.fields.ev, vmcb->eventinj.fields.type,
+ vmcb->eventinj.fields.vector);
printk("exitcode = %#Lx exitintinfo = %#Lx\n",
(unsigned long long)vmcb->exitcode,
(unsigned long long)vmcb->exitintinfo.bytes);

View file

@ -0,0 +1,65 @@
From: Jan Beulich <jbeulich@suse.com>
Subject: xen: fix ioreq handling
Avoid double fetches and bounds check size to avoid overflowing
internal variables.
This is XSA-197.
Reported-by: yanghongke <yanghongke@huawei.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Ian Jackson <ian.jackson@eu.citrix.com>
--- a/i386-dm/helper2.c
+++ b/i386-dm/helper2.c
@@ -375,6 +375,11 @@ static void cpu_ioreq_pio(CPUState *env,
{
uint32_t i;
+ if (req->size > sizeof(unsigned long)) {
+ fprintf(stderr, "PIO: bad size (%u)\n", req->size);
+ exit(-1);
+ }
+
if (req->dir == IOREQ_READ) {
if (!req->data_is_ptr) {
req->data = do_inp(env, req->addr, req->size);
@@ -404,6 +409,11 @@ static void cpu_ioreq_move(CPUState *env
{
uint32_t i;
+ if (req->size > sizeof(req->data)) {
+ fprintf(stderr, "MMIO: bad size (%u)\n", req->size);
+ exit(-1);
+ }
+
if (!req->data_is_ptr) {
if (req->dir == IOREQ_READ) {
for (i = 0; i < req->count; i++) {
@@ -516,11 +526,13 @@ static int __handle_buffered_iopage(CPUS
req.df = 1;
req.type = buf_req->type;
req.data_is_ptr = 0;
+ xen_rmb();
qw = (req.size == 8);
if (qw) {
buf_req = &buffered_io_page->buf_ioreq[(rdptr + 1) %
IOREQ_BUFFER_SLOT_NUM];
req.data |= ((uint64_t)buf_req->data) << 32;
+ xen_rmb();
}
__handle_ioreq(env, &req);
@@ -552,7 +564,11 @@ static void cpu_handle_ioreq(void *opaqu
__handle_buffered_iopage(env);
if (req) {
- __handle_ioreq(env, req);
+ ioreq_t copy = *req;
+
+ xen_rmb();
+ __handle_ioreq(env, &copy);
+ req->data = copy.data;
if (req->state != STATE_IOREQ_INPROCESS) {
fprintf(logfile, "Badness in I/O request ... not in service?!: "

View file

@ -0,0 +1,63 @@
From: Jan Beulich <jbeulich@suse.com>
Subject: xen: fix ioreq handling
Avoid double fetches and bounds check size to avoid overflowing
internal variables.
This is XSA-197.
Reported-by: yanghongke <yanghongke@huawei.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -810,6 +810,10 @@ static void cpu_ioreq_pio(ioreq_t *req)
trace_cpu_ioreq_pio(req, req->dir, req->df, req->data_is_ptr, req->addr,
req->data, req->count, req->size);
+ if (req->size > sizeof(uint32_t)) {
+ hw_error("PIO: bad size (%u)", req->size);
+ }
+
if (req->dir == IOREQ_READ) {
if (!req->data_is_ptr) {
req->data = do_inp(req->addr, req->size);
@@ -846,6 +850,10 @@ static void cpu_ioreq_move(ioreq_t *req)
trace_cpu_ioreq_move(req, req->dir, req->df, req->data_is_ptr, req->addr,
req->data, req->count, req->size);
+ if (req->size > sizeof(req->data)) {
+ hw_error("MMIO: bad size (%u)", req->size);
+ }
+
if (!req->data_is_ptr) {
if (req->dir == IOREQ_READ) {
for (i = 0; i < req->count; i++) {
@@ -1010,11 +1018,13 @@ static int handle_buffered_iopage(XenIOS
req.df = 1;
req.type = buf_req->type;
req.data_is_ptr = 0;
+ xen_rmb();
qw = (req.size == 8);
if (qw) {
buf_req = &buf_page->buf_ioreq[(rdptr + 1) %
IOREQ_BUFFER_SLOT_NUM];
req.data |= ((uint64_t)buf_req->data) << 32;
+ xen_rmb();
}
handle_ioreq(state, &req);
@@ -1045,7 +1055,11 @@ static void cpu_handle_ioreq(void *opaqu
handle_buffered_iopage(state);
if (req) {
- handle_ioreq(state, req);
+ ioreq_t copy = *req;
+
+ xen_rmb();
+ handle_ioreq(state, &copy);
+ req->data = copy.data;
if (req->state != STATE_IOREQ_INPROCESS) {
fprintf(stderr, "Badness in I/O request ... not in service?!: "

View file

@ -0,0 +1,62 @@
From 71a389ae940bc52bf897a6e5becd73fd8ede94c5 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Thu, 3 Nov 2016 16:37:40 +0000
Subject: [PATCH] pygrub: Properly quote results, when returning them to the
caller:
* When the caller wants sexpr output, use `repr()'
This is what Xend expects.
The returned S-expressions are now escaped and quoted by Python,
generally using '...'. Previously kernel and ramdisk were unquoted
and args was quoted with "..." but without proper escaping. This
change may break toolstacks which do not properly dequote the
returned S-expressions.
* When the caller wants "simple" output, crash if the delimiter is
contained in the returned value.
With --output-format=simple it does not seem like this could ever
happen, because the bootloader config parsers all take line-based
input from the various bootloader config files.
With --output-format=simple0, this can happen if the bootloader
config file contains nul bytes.
This is XSA-198.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Tested-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
tools/pygrub/src/pygrub | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tools/pygrub/src/pygrub b/tools/pygrub/src/pygrub
index 40f9584..dd0c8f7 100755
--- a/tools/pygrub/src/pygrub
+++ b/tools/pygrub/src/pygrub
@@ -721,14 +721,17 @@ def sniff_netware(fs, cfg):
return cfg
def format_sxp(kernel, ramdisk, args):
- s = "linux (kernel %s)" % kernel
+ s = "linux (kernel %s)" % repr(kernel)
if ramdisk:
- s += "(ramdisk %s)" % ramdisk
+ s += "(ramdisk %s)" % repr(ramdisk)
if args:
- s += "(args \"%s\")" % args
+ s += "(args %s)" % repr(args)
return s
def format_simple(kernel, ramdisk, args, sep):
+ for check in (kernel, ramdisk, args):
+ if check is not None and sep in check:
+ raise RuntimeError, "simple format cannot represent delimiter-containing value"
s = ("kernel %s" % kernel) + sep
if ramdisk:
s += ("ramdisk %s" % ramdisk) + sep
--
2.1.4