|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH] x86/VMX: sanitize VM86 TSS handling
The present way of setting this up is flawed: Leaving the I/O bitmap
pointer at zero means that the interrupt redirection bitmap lives
outside (ahead of) the allocated space of the TSS. Similarly setting a
TSS limit of 255 when only 128 bytes get allocated means that 128 extra
bytes may be accessed by the CPU during I/O port access processing.
Introduce a new HVM param to set the allocated size of the TSS, and
have the hypervisor actually take care of setting namely the I/O bitmap
pointer. Both this and the segment limit now take the allocated size
into account.
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
TBD: Do we really want to re-init the TSS every time we are about to
use it? This can happen quite often during boot, especially while
grub is running.
--- a/tools/firmware/hvmloader/hvmloader.c
+++ b/tools/firmware/hvmloader/hvmloader.c
@@ -177,18 +177,30 @@ static void cmos_write_memory_size(void)
}
/*
- * Set up an empty TSS area for virtual 8086 mode to use.
- * The only important thing is that it musn't have any bits set
- * in the interrupt redirection bitmap, so all zeros will do.
+ * Set up an empty TSS area for virtual 8086 mode to use. Its content is
+ * going to be managed by Xen, but zero fill it just in case.
*/
static void init_vm86_tss(void)
{
+/*
+ * Have the TSS cover the ISA port range, which makes it
+ * - 104 bytes base structure
+ * - 32 bytes interrupt redirection bitmap
+ * - 128 bytes I/O bitmap
+ * - one trailing byte
+ * or a total of to 265 bytes. As it needs to be be a power of two for
+ * now (or else the alignment parameter to mem_alloc() needs adjustment),
+ * this ends up requiring 512 bytes.
+ */
+#define TSS_SIZE 512
void *tss;
- tss = mem_alloc(128, 128);
- memset(tss, 0, 128);
+ tss = mem_alloc(TSS_SIZE, TSS_SIZE);
+ memset(tss, 0, TSS_SIZE);
hvm_param_set(HVM_PARAM_VM86_TSS, virt_to_phys(tss));
+ hvm_param_set(HVM_PARAM_VM86_TSS_SIZE, TSS_SIZE);
printf("vm86 TSS at %08lx\n", virt_to_phys(tss));
+#undef TSS_SIZE
}
static void apic_setup(void)
--- a/tools/libxc/xc_sr_save_x86_hvm.c
+++ b/tools/libxc/xc_sr_save_x86_hvm.c
@@ -68,6 +68,7 @@ static int write_hvm_params(struct xc_sr
HVM_PARAM_MONITOR_RING_PFN,
HVM_PARAM_SHARING_RING_PFN,
HVM_PARAM_VM86_TSS,
+ HVM_PARAM_VM86_TSS_SIZE,
HVM_PARAM_CONSOLE_PFN,
HVM_PARAM_ACPI_IOPORTS_LOCATION,
HVM_PARAM_VIRIDIAN,
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2850,6 +2850,43 @@ static int hvm_load_segment_selector(
return 1;
}
+struct tss32 {
+ uint16_t back_link, :16;
+ uint32_t esp0;
+ uint16_t ss0, :16;
+ uint32_t esp1;
+ uint16_t ss1, :16;
+ uint32_t esp2;
+ uint16_t ss2, :16;
+ uint32_t cr3, eip, eflags, eax, ecx, edx, ebx, esp, ebp, esi, edi;
+ uint16_t es, :16, cs, :16, ss, :16, ds, :16, fs, :16, gs, :16, ldt, :16;
+ uint16_t trace /* :1 */, iomap;
+};
+
+void hvm_prepare_vm86_tss(struct vcpu *v, uint32_t base, uint32_t limit)
+{
+ /*
+ * If the provided area is large enough to cover at least the ISA port
+ * range, keep the bitmaps outside the base structure. For rather small
+ * areas (namely relevant for guests having been migrated from older
+ * Xen versions), maximize interrupt vector and port coverage by pointing
+ * the I/O bitmap at 0x20 (which puts the interrupt redirection bitmap
+ * right at zero), accepting accesses to port 0x235 (represented by bit 5
+ * of byte 0x46) to trigger #GP (which will simply result in the access
+ * being handled by the emulator via a slightly different path than it
+ * would be anyway). Be sure to include one extra byte at the end of the
+ * I/O bitmap (hence the missing "- 1" in the comparison is not an
+ * off-by-one mistake), which we deliberately don't fill with all ones.
+ */
+ uint16_t iomap = (limit >= sizeof(struct tss32) + (0x100 / 8) + (0x400 / 8)
+ ? sizeof(struct tss32) : 0) + (0x100 / 8);
+
+ ASSERT(limit >= sizeof(struct tss32) - 1);
+ hvm_copy_to_guest_phys(base, NULL, limit + 1, v);
+ hvm_copy_to_guest_phys(base + offsetof(struct tss32, iomap),
+ &iomap, sizeof(iomap), v);
+}
+
void hvm_task_switch(
uint16_t tss_sel, enum hvm_task_switch_reason taskswitch_reason,
int32_t errcode)
@@ -2862,18 +2899,7 @@ void hvm_task_switch(
unsigned int eflags;
pagefault_info_t pfinfo;
int exn_raised, rc;
- struct {
- u16 back_link,__blh;
- u32 esp0;
- u16 ss0, _0;
- u32 esp1;
- u16 ss1, _1;
- u32 esp2;
- u16 ss2, _2;
- u32 cr3, eip, eflags, eax, ecx, edx, ebx, esp, ebp, esi, edi;
- u16 es, _3, cs, _4, ss, _5, ds, _6, fs, _7, gs, _8, ldt, _9;
- u16 trace, iomap;
- } tss;
+ struct tss32 tss;
hvm_get_segment_register(v, x86_seg_gdtr, &gdt);
hvm_get_segment_register(v, x86_seg_tr, &prev_tr);
@@ -4276,6 +4302,7 @@ static int hvm_allow_set_param(struct do
/* The following parameters can be set by the guest. */
case HVM_PARAM_CALLBACK_IRQ:
case HVM_PARAM_VM86_TSS:
+ case HVM_PARAM_VM86_TSS_SIZE:
case HVM_PARAM_ACPI_IOPORTS_LOCATION:
case HVM_PARAM_VM_GENERATION_ID_ADDR:
case HVM_PARAM_STORE_EVTCHN:
@@ -4484,6 +4511,31 @@ static int hvmop_set_param(
}
d->arch.x87_fip_width = a.value;
break;
+
+ case HVM_PARAM_VM86_TSS:
+ /* Hardware would silently truncate high bits. */
+ if ( a.value != (uint32_t)a.value )
+ {
+ if ( d == curr_d )
+ domain_crash(d);
+ rc = -EINVAL;
+ }
+ break;
+
+ case HVM_PARAM_VM86_TSS_SIZE:
+ if ( a.value < sizeof(struct tss32) )
+ {
+ if ( d == curr_d )
+ domain_crash(d);
+ rc = -EINVAL;
+ }
+ /*
+ * Cap at the theoretically useful maximum (base structure plus
+ * 256 bits interrupt redirection bitmap + 64k bits I/O bitmap).
+ */
+ else if ( a.value > sizeof(struct tss32) + (0x100 / 8) + (0x10000 / 8)
)
+ a.value = sizeof(struct tss32) + (0x100 / 8) + (0x10000 / 8);
+ break;
}
if ( rc != 0 )
@@ -4513,6 +4565,7 @@ static int hvm_allow_get_param(struct do
/* The following parameters can be read by the guest. */
case HVM_PARAM_CALLBACK_IRQ:
case HVM_PARAM_VM86_TSS:
+ case HVM_PARAM_VM86_TSS_SIZE:
case HVM_PARAM_ACPI_IOPORTS_LOCATION:
case HVM_PARAM_VM_GENERATION_ID_ADDR:
case HVM_PARAM_STORE_PFN:
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1120,12 +1120,20 @@ static void vmx_set_segment_register(str
if ( seg == x86_seg_tr )
{
- if ( v->domain->arch.hvm_domain.params[HVM_PARAM_VM86_TSS] )
+ const struct domain *d = v->domain;
+
+ if ( d->arch.hvm_domain.params[HVM_PARAM_VM86_TSS] )
{
sel = 0;
attr = vm86_tr_attr;
- limit = 0xff;
- base = v->domain->arch.hvm_domain.params[HVM_PARAM_VM86_TSS];
+ /*
+ * Old hvmloader binaries hardcode the size to 128 bytes,
+ * without setting HVM_PARAM_VM86_TSS_SIZE.
+ */
+ limit = (d->arch.hvm_domain.params[HVM_PARAM_VM86_TSS_SIZE]
+ ?: 0x80) - 1;
+ base = d->arch.hvm_domain.params[HVM_PARAM_VM86_TSS];
+ hvm_prepare_vm86_tss(v, base, limit);
v->arch.hvm_vmx.vm86_segment_mask &= ~(1u << seg);
}
else
--- a/xen/include/asm-x86/hvm/support.h
+++ b/xen/include/asm-x86/hvm/support.h
@@ -111,6 +111,8 @@ int hvm_do_hypercall(struct cpu_user_reg
void hvm_hlt(unsigned int eflags);
void hvm_triple_fault(void);
+void hvm_prepare_vm86_tss(struct vcpu *v, uint32_t base, uint32_t limit);
+
void hvm_rdtsc_intercept(struct cpu_user_regs *regs);
int __must_check hvm_handle_xsetbv(u32 index, u64 new_bv);
--- a/xen/include/public/hvm/params.h
+++ b/xen/include/public/hvm/params.h
@@ -253,6 +253,9 @@
*/
#define HVM_PARAM_X87_FIP_WIDTH 36
-#define HVM_NR_PARAMS 37
+/* Size of TSS used on Intel when CR0.PE=0. */
+#define HVM_PARAM_VM86_TSS_SIZE 37
+
+#define HVM_NR_PARAMS 38
#endif /* __XEN_PUBLIC_HVM_PARAMS_H__ */
Attachment:
x86-HVM-VM86-TSS.patch _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |