[prev in list] [next in list] [prev in thread] [next in thread]
List: oss-security
Subject: [oss-security] Xen Security Advisory 261 (CVE-2018-10982) - x86 vHPET interrupt injection errors
From: Xen.org security team <security () xen ! org>
Date: 2018-05-11 10:13:18
Message-ID: E1fH538-0003a6-AA () xenbits ! xenproject ! org
[Download RAW message or body]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Xen Security Advisory CVE-2018-10982 / XSA-261
version 3
x86 vHPET interrupt injection errors
UPDATES IN VERSION 3
====================
CVE assigned.
ISSUE DESCRIPTION
=================
The High Precision Event Timer (HPET) can be configured to deliver
interrupts in one of three different modes - through legacy interrupts;
through the IO-APIC; or optionally via a method similar to PCI MSI. The
last mode is optional and not implemented by Xen. However, of the first
two modes, only the legacy variant was properly implemented.
If a guest set up an HPET timer in IO-APIC mode, Xen would still
handle this using the code for the legacy mode. Unfortunately, the
available IO-APIC mode interrupt numbers are higher than legacy mode
interrupts. The result was array overruns.
IMPACT
======
A malicious or buggy HVM guest may cause a hypervisor crash, resulting
in a Denial of Service (DoS) affecting the entire host. Privilege
escalation, or information leaks, cannot be excluded.
VULNERABLE SYSTEMS
==================
Xen versions 3.4 and later are vulnerable.
Only x86 systems are vulnerable. ARM systems are not vulnerable.
Only x86 HVM guests can exploit the vulnerability. x86 PV and PVH
guests cannot exploit the vulnerability.
Only x86 HVM guests provided with hypervisor-side HPET emulation can
exploit the vulnerability. That is the default configuration. x86
HVM guests whose configuration explicitly disables this emulation (via
"hpet=0") cannot exploit the vulnerability.
MITIGATION
==========
Running only PV or PVH guests avoids the vulnerability.
Not exposing the hypervisor based HPET emulation to HVM guests, by
adding "hpet=0" to the guest configuration, also avoids the
vulnerability.
CREDITS
=======
This issue was discovered by Roger Pau Monné of Citrix.
RESOLUTION
==========
Applying the appropriate attached patch resolves this issue.
xsa261.patch xen-unstable, Xen 4.10.x
xsa261-4.9.patch Xen 4.9.x
xsa261-4.8.patch Xen 4.8.x
xsa261-4.7.patch Xen 4.7.x, Xen 4.6.x
$ sha256sum xsa261*
7b7bbf0fb497491911816e522902f72d3b41355ba71455ab82ebf980160d1a1f xsa261.meta
175501977204db84d08a6fd81d9fd4b69f97f70cbf6f65e6ce0abfeab03eae95 xsa261.patch
98fb28bac871aae7c2f897a5506a2b03f340bf122a3a7f65aa65f3b3c9a525b4 xsa261-4.7.patch
503f1476813e6572dc37b5a0df65b5390567230d9cc006752bf72bf57bbd754d xsa261-4.8.patch
f1aac841327d3b5b1e2007b4ebe56223de488e1eb2fa636653725d7d7cd5f82a xsa261-4.9.patch
$
DEPLOYMENT DURING EMBARGO
=========================
Deployment of the patches described above (or others which are
substantially similar) and the PV/PVH guest mitigation are permitted
during the embargo, even on public-facing systems with untrusted guest
users and administrators.
HOWEVER deployment of the "hpet=0" guest config mitigation described
above is NOT permitted (except where all the affected systems and VMs
are administered and used only by organisations which are members of
the Xen Project Security Issues Predisclosure List). Specifically,
deployment on public cloud systems is NOT permitted.
This is because in that case the configuration change is visible to the
guest, which could lead to the rediscovery of the vulnerability.
But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).
Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.
(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable. This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)
For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQEcBAEBCAAGBQJa9Wy1AAoJEIP+FMlX6CvZaxkIALwHLRw4JlORTplsS9bwnioh
kuNausNp1pU9IqfcUKEI17n5+HekiXfLNennHEWYgYfdpNlWAbjUW5GaczII0KmS
IJa8UvptnYydhg73Q8WWlYOx3i8nS15+ioIH8RIa1Vtvv0p7vbHf8C9BmjmYf1oa
5WH9Ut4Sx5wwALuCh/gO71ja5vgAAIpgQTf5R4KL0x9sJiCLTw2A4yxVmVd24bES
1fNoH3/qdbjgMjl7sLPCdsXLOqg9Xi77i5f5XnJMZgWQRQyh0XLeo5itiDIuMF/k
tEMuEpKQ5+t4GNg92B67dFVWxeX1VIRrQ9a18WfXcwttM3xLFNcqt3BpSV9K8Tg=
=KeNf
-----END PGP SIGNATURE-----
["xsa261.meta" (application/octet-stream)]
["xsa261.patch" (application/octet-stream)]
From: Xen Project Security Team <security@xenproject.org>
Subject: x86/vpt: add support for IO-APIC routed interrupts
And modify the HPET code to make use of it. Currently HPET interrupts
are always treated as ISA and thus injected through the vPIC. This is
wrong because HPET interrupts when not in legacy mode should be
injected from the IO-APIC.
To make things worse, the supported interrupt routing values are set
to [20..23], which clearly falls outside of the ISA range, thus
leading to an ASSERT in debug builds or memory corruption in non-debug
builds because the interrupt injection code will write out of the
bounds of the arch.hvm_domain.vpic array.
Since the HPET interrupt source can change between ISA and IO-APIC
always destroy the timer before changing the mode, or else Xen risks
changing it while the timer is active.
Note that vpt interrupt injection is racy in the sense that the
vIO-APIC RTE entry can be written by the guest in between the call to
pt_irq_masked and hvm_ioapic_assert, or the call to pt_update_irq and
pt_intr_post. Those are not deemed to be security issues, but rather
quirks of the current implementation. In the worse case the guest
might lose interrupts or get multiple interrupt vectors injected for
the same timer source.
This is part of XSA-261.
Address actual and potential compiler warnings. Fix formatting.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Changes since v2:
- Move fallthrough comment to be just above the case label.
- Fix now stale comment in pt_update_irq.
- Use NR_ISAIRQS instead of 16.
- Expand commit message to mention the quirkiness of vpt interrupt
injection.
Changes since v1:
- Simply usage of gsi in pt_irq_masked.
- Introduce hvm_ioapic_assert.
- Fix pt->source == PTSRC_isa in create_periodic_time.
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -264,13 +264,20 @@ static void hpet_set_timer(HPETState *h,
diff = (timer_is_32bit(h, tn) && (-diff > HPET_TINY_TIME_SPAN))
? (uint32_t)diff : 0;
+ destroy_periodic_time(&h->pt[tn]);
if ( (tn <= 1) && (h->hpet.config & HPET_CFG_LEGACY) )
+ {
/* if LegacyReplacementRoute bit is set, HPET specification requires
timer0 be routed to IRQ0 in NON-APIC or IRQ2 in the I/O APIC,
timer1 be routed to IRQ8 in NON-APIC or IRQ8 in the I/O APIC. */
irq = (tn == 0) ? 0 : 8;
+ h->pt[tn].source = PTSRC_isa;
+ }
else
+ {
irq = timer_int_route(h, tn);
+ h->pt[tn].source = PTSRC_ioapic;
+ }
/*
* diff is the time from now when the timer should fire, for a periodic
--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -41,6 +41,26 @@ static void assert_gsi(struct domain *d,
vioapic_irq_positive_edge(d, ioapic_gsi);
}
+int hvm_ioapic_assert(struct domain *d, unsigned int gsi, bool level)
+{
+ struct hvm_irq *hvm_irq = hvm_domain_irq(d);
+ int vector;
+
+ if ( gsi >= hvm_irq->nr_gsis )
+ {
+ ASSERT_UNREACHABLE();
+ return -1;
+ }
+
+ spin_lock(&d->arch.hvm_domain.irq_lock);
+ if ( !level || hvm_irq->gsi_assert_count[gsi]++ == 0 )
+ assert_gsi(d, gsi);
+ vector = vioapic_get_vector(d, gsi);
+ spin_unlock(&d->arch.hvm_domain.irq_lock);
+
+ return vector;
+}
+
static void assert_irq(struct domain *d, unsigned ioapic_gsi, unsigned pic_irq)
{
assert_gsi(d, ioapic_gsi);
--- a/xen/arch/x86/hvm/vpt.c
+++ b/xen/arch/x86/hvm/vpt.c
@@ -107,31 +107,49 @@ static int pt_irq_vector(struct periodic
static int pt_irq_masked(struct periodic_time *pt)
{
struct vcpu *v = pt->vcpu;
- unsigned int gsi, isa_irq;
- int mask;
- uint8_t pic_imr;
+ unsigned int gsi = pt->irq;
- if ( pt->source == PTSRC_lapic )
+ switch ( pt->source )
+ {
+ case PTSRC_lapic:
{
struct vlapic *vlapic = vcpu_vlapic(v);
+
return (!vlapic_enabled(vlapic) ||
(vlapic_get_reg(vlapic, APIC_LVTT) & APIC_LVT_MASKED));
}
- isa_irq = pt->irq;
- gsi = hvm_isa_irq_to_gsi(isa_irq);
- pic_imr = v->domain->arch.hvm_domain.vpic[isa_irq >> 3].imr;
- mask = vioapic_get_mask(v->domain, gsi);
- if ( mask < 0 )
- {
- dprintk(XENLOG_WARNING, "d%u: invalid GSI (%u) for platform timer\n",
- v->domain->domain_id, gsi);
- domain_crash(v->domain);
- return -1;
+ case PTSRC_isa:
+ {
+ uint8_t pic_imr = v->domain->arch.hvm_domain.vpic[pt->irq >> 3].imr;
+
+ /* Check if the interrupt is unmasked in the PIC. */
+ if ( !(pic_imr & (1 << (pt->irq & 7))) && vlapic_accept_pic_intr(v) )
+ return 0;
+
+ gsi = hvm_isa_irq_to_gsi(pt->irq);
+ }
+
+ /* Fallthrough to check if the interrupt is masked on the IO APIC. */
+ case PTSRC_ioapic:
+ {
+ int mask = vioapic_get_mask(v->domain, gsi);
+
+ if ( mask < 0 )
+ {
+ dprintk(XENLOG_WARNING,
+ "d%d: invalid GSI (%u) for platform timer\n",
+ v->domain->domain_id, gsi);
+ domain_crash(v->domain);
+ return -1;
+ }
+
+ return mask;
+ }
}
- return (((pic_imr & (1 << (isa_irq & 7))) || !vlapic_accept_pic_intr(v)) &&
- mask);
+ ASSERT_UNREACHABLE();
+ return 1;
}
static void pt_lock(struct periodic_time *pt)
@@ -252,7 +270,7 @@ int pt_update_irq(struct vcpu *v)
struct list_head *head = &v->arch.hvm_vcpu.tm_list;
struct periodic_time *pt, *temp, *earliest_pt;
uint64_t max_lag;
- int irq, is_lapic, pt_vector;
+ int irq, pt_vector = -1;
spin_lock(&v->arch.hvm_vcpu.tm_lock);
@@ -288,29 +306,26 @@ int pt_update_irq(struct vcpu *v)
earliest_pt->irq_issued = 1;
irq = earliest_pt->irq;
- is_lapic = (earliest_pt->source == PTSRC_lapic);
spin_unlock(&v->arch.hvm_vcpu.tm_lock);
- /*
- * If periodic timer interrut is handled by lapic, its vector in
- * IRR is returned and used to set eoi_exit_bitmap for virtual
- * interrupt delivery case. Otherwise return -1 to do nothing.
- */
- if ( is_lapic )
+ switch ( earliest_pt->source )
{
+ case PTSRC_lapic:
+ /*
+ * If periodic timer interrupt is handled by lapic, its vector in
+ * IRR is returned and used to set eoi_exit_bitmap for virtual
+ * interrupt delivery case. Otherwise return -1 to do nothing.
+ */
vlapic_set_irq(vcpu_vlapic(v), irq, 0);
pt_vector = irq;
- }
- else
- {
+ break;
+
+ case PTSRC_isa:
hvm_isa_irq_deassert(v->domain, irq);
if ( platform_legacy_irq(irq) && vlapic_accept_pic_intr(v) &&
v->domain->arch.hvm_domain.vpic[irq >> 3].int_output )
- {
hvm_isa_irq_assert(v->domain, irq, NULL);
- pt_vector = -1;
- }
else
{
pt_vector = hvm_isa_irq_assert(v->domain, irq, vioapic_get_vector);
@@ -321,6 +336,17 @@ int pt_update_irq(struct vcpu *v)
if ( pt_vector < 0 || !vlapic_test_irq(vcpu_vlapic(v), pt_vector) )
pt_vector = -1;
}
+ break;
+
+ case PTSRC_ioapic:
+ /*
+ * NB: At the moment IO-APIC routed interrupts generated by vpt devices
+ * (HPET) are edge-triggered.
+ */
+ pt_vector = hvm_ioapic_assert(v->domain, irq, false);
+ if ( pt_vector < 0 || !vlapic_test_irq(vcpu_vlapic(v), pt_vector) )
+ pt_vector = -1;
+ break;
}
return pt_vector;
@@ -418,7 +444,14 @@ void create_periodic_time(
struct vcpu *v, struct periodic_time *pt, uint64_t delta,
uint64_t period, uint8_t irq, time_cb *cb, void *data)
{
- ASSERT(pt->source != 0);
+ if ( !pt->source ||
+ (pt->irq >= NR_ISAIRQS && pt->source == PTSRC_isa) ||
+ (pt->irq >= hvm_domain_irq(v->domain)->nr_gsis &&
+ pt->source == PTSRC_ioapic) )
+ {
+ ASSERT_UNREACHABLE();
+ return;
+ }
destroy_periodic_time(pt);
@@ -498,7 +531,7 @@ static void pt_adjust_vcpu(struct period
{
int on_list;
- ASSERT(pt->source == PTSRC_isa);
+ ASSERT(pt->source == PTSRC_isa || pt->source == PTSRC_ioapic);
if ( pt->vcpu == NULL )
return;
--- a/xen/include/asm-x86/hvm/irq.h
+++ b/xen/include/asm-x86/hvm/irq.h
@@ -207,6 +207,9 @@ int hvm_set_pci_link_route(struct domain
int hvm_inject_msi(struct domain *d, uint64_t addr, uint32_t data);
+/* Assert an IO APIC pin. */
+int hvm_ioapic_assert(struct domain *d, unsigned int gsi, bool level);
+
void hvm_maybe_deassert_evtchn_irq(void);
void hvm_assert_evtchn_irq(struct vcpu *v);
void hvm_set_callback_via(struct domain *d, uint64_t via);
--- a/xen/include/asm-x86/hvm/vpt.h
+++ b/xen/include/asm-x86/hvm/vpt.h
@@ -44,6 +44,7 @@ struct periodic_time {
bool_t warned_timeout_too_short;
#define PTSRC_isa 1 /* ISA time source */
#define PTSRC_lapic 2 /* LAPIC time source */
+#define PTSRC_ioapic 3 /* IOAPIC time source */
u8 source; /* PTSRC_ */
u8 irq;
struct vcpu *vcpu; /* vcpu timer interrupt delivers to */
["xsa261-4.7.patch" (application/octet-stream)]
From 0ce5019f87abe20d1e714f8d983418ecfb0ad5df Mon Sep 17 00:00:00 2001
From: Xen Project Security Team <security@xenproject.org>
Date: Mon, 23 Apr 2018 16:56:47 +0100
Subject: [PATCH] x86/vpt: add support for IO-APIC routed interrupts
And modify the HPET code to make use of it. Currently HPET interrupts
are always treated as ISA and thus injected through the vPIC. This is
wrong because HPET interrupts when not in legacy mode should be
injected from the IO-APIC.
To make things worse, the supported interrupt routing values are set
to [20..23], which clearly falls outside of the ISA range, thus
leading to an ASSERT in debug builds or memory corruption in non-debug
builds because the interrupt injection code will write out of the
bounds of the arch.hvm_domain.vpic array.
Since the HPET interrupt source can change between ISA and IO-APIC
always destroy the timer before changing the mode, or else Xen risks
changing it while the timer is active.
Note that vpt interrupt injection is racy in the sense that the
vIO-APIC RTE entry can be written by the guest in between the call to
pt_irq_masked and hvm_ioapic_assert, or the call to pt_update_irq and
pt_intr_post. Those are not deemed to be security issues, but rather
quirks of the current implementation. In the worse case the guest
might lose interrupts or get multiple interrupt vectors injected for
the same timer source.
This is part of XSA-261.
Address actual and potential compiler warnings. Fix formatting.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
xen/arch/x86/hvm/hpet.c | 7 ++++
xen/arch/x86/hvm/irq.c | 19 ++++++++++
xen/arch/x86/hvm/vpt.c | 85 ++++++++++++++++++++++++++++++-------------
xen/include/asm-x86/hvm/irq.h | 3 ++
xen/include/asm-x86/hvm/vpt.h | 1 +
5 files changed, 89 insertions(+), 26 deletions(-)
diff --git a/xen/arch/x86/hvm/hpet.c b/xen/arch/x86/hvm/hpet.c
index f7aed7f..2837709 100644
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -264,13 +264,20 @@ static void hpet_set_timer(HPETState *h, unsigned int tn,
diff = (timer_is_32bit(h, tn) && (-diff > HPET_TINY_TIME_SPAN))
? (uint32_t)diff : 0;
+ destroy_periodic_time(&h->pt[tn]);
if ( (tn <= 1) && (h->hpet.config & HPET_CFG_LEGACY) )
+ {
/* if LegacyReplacementRoute bit is set, HPET specification requires
timer0 be routed to IRQ0 in NON-APIC or IRQ2 in the I/O APIC,
timer1 be routed to IRQ8 in NON-APIC or IRQ8 in the I/O APIC. */
irq = (tn == 0) ? 0 : 8;
+ h->pt[tn].source = PTSRC_isa;
+ }
else
+ {
irq = timer_int_route(h, tn);
+ h->pt[tn].source = PTSRC_ioapic;
+ }
/*
* diff is the time from now when the timer should fire, for a periodic
diff --git a/xen/arch/x86/hvm/irq.c b/xen/arch/x86/hvm/irq.c
index be1d4e8..2d1990b 100644
--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -42,6 +42,25 @@ static void assert_gsi(struct domain *d, unsigned ioapic_gsi)
vioapic_irq_positive_edge(d, ioapic_gsi);
}
+int hvm_ioapic_assert(struct domain *d, unsigned int gsi, bool_t level)
+{
+ int vector;
+
+ if ( gsi >= VIOAPIC_NUM_PINS )
+ {
+ ASSERT_UNREACHABLE();
+ return -1;
+ }
+
+ spin_lock(&d->arch.hvm_domain.irq_lock);
+ if ( !level || d->arch.hvm_domain.irq.gsi_assert_count[gsi]++ == 0 )
+ assert_gsi(d, gsi);
+ vector = domain_vioapic(d)->redirtbl[gsi].fields.vector;
+ spin_unlock(&d->arch.hvm_domain.irq_lock);
+
+ return vector;
+}
+
static void assert_irq(struct domain *d, unsigned ioapic_gsi, unsigned pic_irq)
{
assert_gsi(d, ioapic_gsi);
diff --git a/xen/arch/x86/hvm/vpt.c b/xen/arch/x86/hvm/vpt.c
index 358ec57..f0fe31c 100644
--- a/xen/arch/x86/hvm/vpt.c
+++ b/xen/arch/x86/hvm/vpt.c
@@ -97,22 +97,36 @@ static int pt_irq_vector(struct periodic_time *pt, enum hvm_intsrc src)
static int pt_irq_masked(struct periodic_time *pt)
{
struct vcpu *v = pt->vcpu;
- unsigned int gsi, isa_irq;
- uint8_t pic_imr;
+ unsigned int gsi = pt->irq;
- if ( pt->source == PTSRC_lapic )
+ switch ( pt->source )
+ {
+ case PTSRC_lapic:
{
struct vlapic *vlapic = vcpu_vlapic(v);
+
return (!vlapic_enabled(vlapic) ||
(vlapic_get_reg(vlapic, APIC_LVTT) & APIC_LVT_MASKED));
}
- isa_irq = pt->irq;
- gsi = hvm_isa_irq_to_gsi(isa_irq);
- pic_imr = v->domain->arch.hvm_domain.vpic[isa_irq >> 3].imr;
+ case PTSRC_isa:
+ {
+ uint8_t pic_imr = v->domain->arch.hvm_domain.vpic[pt->irq >> 3].imr;
- return (((pic_imr & (1 << (isa_irq & 7))) || !vlapic_accept_pic_intr(v)) &&
- domain_vioapic(v->domain)->redirtbl[gsi].fields.mask);
+ /* Check if the interrupt is unmasked in the PIC. */
+ if ( !(pic_imr & (1 << (pt->irq & 7))) && vlapic_accept_pic_intr(v) )
+ return 0;
+
+ gsi = hvm_isa_irq_to_gsi(pt->irq);
+ }
+
+ /* Fallthrough to check if the interrupt is masked on the IO APIC. */
+ case PTSRC_ioapic:
+ return domain_vioapic(v->domain)->redirtbl[gsi].fields.mask;
+ }
+
+ ASSERT_UNREACHABLE();
+ return 1;
}
static void pt_lock(struct periodic_time *pt)
@@ -233,7 +247,7 @@ int pt_update_irq(struct vcpu *v)
struct list_head *head = &v->arch.hvm_vcpu.tm_list;
struct periodic_time *pt, *temp, *earliest_pt;
uint64_t max_lag;
- int irq, is_lapic;
+ int irq, pt_vector = -1;
spin_lock(&v->arch.hvm_vcpu.tm_lock);
@@ -269,29 +283,42 @@ int pt_update_irq(struct vcpu *v)
earliest_pt->irq_issued = 1;
irq = earliest_pt->irq;
- is_lapic = (earliest_pt->source == PTSRC_lapic);
spin_unlock(&v->arch.hvm_vcpu.tm_lock);
- if ( is_lapic )
- vlapic_set_irq(vcpu_vlapic(v), irq, 0);
- else
+ switch ( earliest_pt->source )
{
+ case PTSRC_lapic:
+ /*
+ * If periodic timer interrupt is handled by lapic, its vector in
+ * IRR is returned and used to set eoi_exit_bitmap for virtual
+ * interrupt delivery case. Otherwise return -1 to do nothing.
+ */
+ vlapic_set_irq(vcpu_vlapic(v), irq, 0);
+ pt_vector = irq;
+ break;
+
+ case PTSRC_isa:
hvm_isa_irq_deassert(v->domain, irq);
hvm_isa_irq_assert(v->domain, irq);
+
+ if ( platform_legacy_irq(irq) && vlapic_accept_pic_intr(v) &&
+ v->domain->arch.hvm_domain.vpic[irq >> 3].int_output )
+ return -1;
+
+ pt_vector = pt_irq_vector(earliest_pt, hvm_intsrc_lapic);
+ break;
+
+ case PTSRC_ioapic:
+ /*
+ * NB: At the moment IO-APIC routed interrupts generated by vpt devices
+ * (HPET) are edge-triggered.
+ */
+ pt_vector = hvm_ioapic_assert(v->domain, irq, 0);
+ break;
}
- /*
- * If periodic timer interrut is handled by lapic, its vector in
- * IRR is returned and used to set eoi_exit_bitmap for virtual
- * interrupt delivery case. Otherwise return -1 to do nothing.
- */
- if ( !is_lapic &&
- platform_legacy_irq(irq) && vlapic_accept_pic_intr(v) &&
- (&v->domain->arch.hvm_domain)->vpic[irq >> 3].int_output )
- return -1;
- else
- return pt_irq_vector(earliest_pt, hvm_intsrc_lapic);
+ return pt_vector;
}
static struct periodic_time *is_pt_irq(
@@ -386,7 +413,13 @@ void create_periodic_time(
struct vcpu *v, struct periodic_time *pt, uint64_t delta,
uint64_t period, uint8_t irq, time_cb *cb, void *data)
{
- ASSERT(pt->source != 0);
+ if ( !pt->source ||
+ (pt->irq >= NR_ISAIRQS && pt->source == PTSRC_isa) ||
+ (pt->irq >= VIOAPIC_NUM_PINS && pt->source == PTSRC_ioapic) )
+ {
+ ASSERT_UNREACHABLE();
+ return;
+ }
destroy_periodic_time(pt);
@@ -466,7 +499,7 @@ static void pt_adjust_vcpu(struct periodic_time *pt, struct vcpu *v)
{
int on_list;
- ASSERT(pt->source == PTSRC_isa);
+ ASSERT(pt->source == PTSRC_isa || pt->source == PTSRC_ioapic);
if ( pt->vcpu == NULL )
return;
diff --git a/xen/include/asm-x86/hvm/irq.h b/xen/include/asm-x86/hvm/irq.h
index 73b8fb0..c60036b 100644
--- a/xen/include/asm-x86/hvm/irq.h
+++ b/xen/include/asm-x86/hvm/irq.h
@@ -103,4 +103,7 @@ struct hvm_intack hvm_vcpu_has_pending_irq(struct vcpu *v);
struct hvm_intack hvm_vcpu_ack_pending_irq(struct vcpu *v,
struct hvm_intack intack);
+/* Assert an IO APIC pin. */
+int hvm_ioapic_assert(struct domain *d, unsigned int gsi, bool_t level);
+
#endif /* __ASM_X86_HVM_IRQ_H__ */
diff --git a/xen/include/asm-x86/hvm/vpt.h b/xen/include/asm-x86/hvm/vpt.h
index a27bea4..616f040 100644
--- a/xen/include/asm-x86/hvm/vpt.h
+++ b/xen/include/asm-x86/hvm/vpt.h
@@ -45,6 +45,7 @@ struct periodic_time {
bool_t warned_timeout_too_short;
#define PTSRC_isa 1 /* ISA time source */
#define PTSRC_lapic 2 /* LAPIC time source */
+#define PTSRC_ioapic 3 /* IOAPIC time source */
u8 source; /* PTSRC_ */
u8 irq;
struct vcpu *vcpu; /* vcpu timer interrupt delivers to */
--
2.1.4
["xsa261-4.8.patch" (application/octet-stream)]
From: Xen Project Security Team <security@xenproject.org>
Subject: x86/vpt: add support for IO-APIC routed interrupts
And modify the HPET code to make use of it. Currently HPET interrupts
are always treated as ISA and thus injected through the vPIC. This is
wrong because HPET interrupts when not in legacy mode should be
injected from the IO-APIC.
To make things worse, the supported interrupt routing values are set
to [20..23], which clearly falls outside of the ISA range, thus
leading to an ASSERT in debug builds or memory corruption in non-debug
builds because the interrupt injection code will write out of the
bounds of the arch.hvm_domain.vpic array.
Since the HPET interrupt source can change between ISA and IO-APIC
always destroy the timer before changing the mode, or else Xen risks
changing it while the timer is active.
Note that vpt interrupt injection is racy in the sense that the
vIO-APIC RTE entry can be written by the guest in between the call to
pt_irq_masked and hvm_ioapic_assert, or the call to pt_update_irq and
pt_intr_post. Those are not deemed to be security issues, but rather
quirks of the current implementation. In the worse case the guest
might lose interrupts or get multiple interrupt vectors injected for
the same timer source.
This is part of XSA-261.
Address actual and potential compiler warnings. Fix formatting.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -264,13 +264,20 @@ static void hpet_set_timer(HPETState *h,
diff = (timer_is_32bit(h, tn) && (-diff > HPET_TINY_TIME_SPAN))
? (uint32_t)diff : 0;
+ destroy_periodic_time(&h->pt[tn]);
if ( (tn <= 1) && (h->hpet.config & HPET_CFG_LEGACY) )
+ {
/* if LegacyReplacementRoute bit is set, HPET specification requires
timer0 be routed to IRQ0 in NON-APIC or IRQ2 in the I/O APIC,
timer1 be routed to IRQ8 in NON-APIC or IRQ8 in the I/O APIC. */
irq = (tn == 0) ? 0 : 8;
+ h->pt[tn].source = PTSRC_isa;
+ }
else
+ {
irq = timer_int_route(h, tn);
+ h->pt[tn].source = PTSRC_ioapic;
+ }
/*
* diff is the time from now when the timer should fire, for a periodic
--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -42,6 +42,25 @@ static void assert_gsi(struct domain *d,
vioapic_irq_positive_edge(d, ioapic_gsi);
}
+int hvm_ioapic_assert(struct domain *d, unsigned int gsi, bool level)
+{
+ int vector;
+
+ if ( gsi >= VIOAPIC_NUM_PINS )
+ {
+ ASSERT_UNREACHABLE();
+ return -1;
+ }
+
+ spin_lock(&d->arch.hvm_domain.irq_lock);
+ if ( !level || d->arch.hvm_domain.irq.gsi_assert_count[gsi]++ == 0 )
+ assert_gsi(d, gsi);
+ vector = domain_vioapic(d)->redirtbl[gsi].fields.vector;
+ spin_unlock(&d->arch.hvm_domain.irq_lock);
+
+ return vector;
+}
+
static void assert_irq(struct domain *d, unsigned ioapic_gsi, unsigned pic_irq)
{
assert_gsi(d, ioapic_gsi);
--- a/xen/arch/x86/hvm/vpt.c
+++ b/xen/arch/x86/hvm/vpt.c
@@ -97,22 +97,36 @@ static int pt_irq_vector(struct periodic
static int pt_irq_masked(struct periodic_time *pt)
{
struct vcpu *v = pt->vcpu;
- unsigned int gsi, isa_irq;
- uint8_t pic_imr;
+ unsigned int gsi = pt->irq;
- if ( pt->source == PTSRC_lapic )
+ switch ( pt->source )
+ {
+ case PTSRC_lapic:
{
struct vlapic *vlapic = vcpu_vlapic(v);
+
return (!vlapic_enabled(vlapic) ||
(vlapic_get_reg(vlapic, APIC_LVTT) & APIC_LVT_MASKED));
}
- isa_irq = pt->irq;
- gsi = hvm_isa_irq_to_gsi(isa_irq);
- pic_imr = v->domain->arch.hvm_domain.vpic[isa_irq >> 3].imr;
+ case PTSRC_isa:
+ {
+ uint8_t pic_imr = v->domain->arch.hvm_domain.vpic[pt->irq >> 3].imr;
- return (((pic_imr & (1 << (isa_irq & 7))) || !vlapic_accept_pic_intr(v)) &&
- domain_vioapic(v->domain)->redirtbl[gsi].fields.mask);
+ /* Check if the interrupt is unmasked in the PIC. */
+ if ( !(pic_imr & (1 << (pt->irq & 7))) && vlapic_accept_pic_intr(v) )
+ return 0;
+
+ gsi = hvm_isa_irq_to_gsi(pt->irq);
+ }
+
+ /* Fallthrough to check if the interrupt is masked on the IO APIC. */
+ case PTSRC_ioapic:
+ return domain_vioapic(v->domain)->redirtbl[gsi].fields.mask;
+ }
+
+ ASSERT_UNREACHABLE();
+ return 1;
}
static void pt_lock(struct periodic_time *pt)
@@ -233,7 +247,7 @@ int pt_update_irq(struct vcpu *v)
struct list_head *head = &v->arch.hvm_vcpu.tm_list;
struct periodic_time *pt, *temp, *earliest_pt;
uint64_t max_lag;
- int irq, is_lapic;
+ int irq, pt_vector = -1;
spin_lock(&v->arch.hvm_vcpu.tm_lock);
@@ -269,29 +283,42 @@ int pt_update_irq(struct vcpu *v)
earliest_pt->irq_issued = 1;
irq = earliest_pt->irq;
- is_lapic = (earliest_pt->source == PTSRC_lapic);
spin_unlock(&v->arch.hvm_vcpu.tm_lock);
- if ( is_lapic )
- vlapic_set_irq(vcpu_vlapic(v), irq, 0);
- else
+ switch ( earliest_pt->source )
{
+ case PTSRC_lapic:
+ /*
+ * If periodic timer interrupt is handled by lapic, its vector in
+ * IRR is returned and used to set eoi_exit_bitmap for virtual
+ * interrupt delivery case. Otherwise return -1 to do nothing.
+ */
+ vlapic_set_irq(vcpu_vlapic(v), irq, 0);
+ pt_vector = irq;
+ break;
+
+ case PTSRC_isa:
hvm_isa_irq_deassert(v->domain, irq);
hvm_isa_irq_assert(v->domain, irq);
+
+ if ( platform_legacy_irq(irq) && vlapic_accept_pic_intr(v) &&
+ v->domain->arch.hvm_domain.vpic[irq >> 3].int_output )
+ return -1;
+
+ pt_vector = pt_irq_vector(earliest_pt, hvm_intsrc_lapic);
+ break;
+
+ case PTSRC_ioapic:
+ /*
+ * NB: At the moment IO-APIC routed interrupts generated by vpt devices
+ * (HPET) are edge-triggered.
+ */
+ pt_vector = hvm_ioapic_assert(v->domain, irq, false);
+ break;
}
- /*
- * If periodic timer interrut is handled by lapic, its vector in
- * IRR is returned and used to set eoi_exit_bitmap for virtual
- * interrupt delivery case. Otherwise return -1 to do nothing.
- */
- if ( !is_lapic &&
- platform_legacy_irq(irq) && vlapic_accept_pic_intr(v) &&
- (&v->domain->arch.hvm_domain)->vpic[irq >> 3].int_output )
- return -1;
- else
- return pt_irq_vector(earliest_pt, hvm_intsrc_lapic);
+ return pt_vector;
}
static struct periodic_time *is_pt_irq(
@@ -386,7 +413,13 @@ void create_periodic_time(
struct vcpu *v, struct periodic_time *pt, uint64_t delta,
uint64_t period, uint8_t irq, time_cb *cb, void *data)
{
- ASSERT(pt->source != 0);
+ if ( !pt->source ||
+ (pt->irq >= NR_ISAIRQS && pt->source == PTSRC_isa) ||
+ (pt->irq >= VIOAPIC_NUM_PINS && pt->source == PTSRC_ioapic) )
+ {
+ ASSERT_UNREACHABLE();
+ return;
+ }
destroy_periodic_time(pt);
@@ -466,7 +499,7 @@ static void pt_adjust_vcpu(struct period
{
int on_list;
- ASSERT(pt->source == PTSRC_isa);
+ ASSERT(pt->source == PTSRC_isa || pt->source == PTSRC_ioapic);
if ( pt->vcpu == NULL )
return;
--- a/xen/include/asm-x86/hvm/irq.h
+++ b/xen/include/asm-x86/hvm/irq.h
@@ -103,4 +103,7 @@ struct hvm_intack hvm_vcpu_has_pending_i
struct hvm_intack hvm_vcpu_ack_pending_irq(struct vcpu *v,
struct hvm_intack intack);
+/* Assert an IO APIC pin. */
+int hvm_ioapic_assert(struct domain *d, unsigned int gsi, bool level);
+
#endif /* __ASM_X86_HVM_IRQ_H__ */
--- a/xen/include/asm-x86/hvm/vpt.h
+++ b/xen/include/asm-x86/hvm/vpt.h
@@ -45,6 +45,7 @@ struct periodic_time {
bool_t warned_timeout_too_short;
#define PTSRC_isa 1 /* ISA time source */
#define PTSRC_lapic 2 /* LAPIC time source */
+#define PTSRC_ioapic 3 /* IOAPIC time source */
u8 source; /* PTSRC_ */
u8 irq;
struct vcpu *vcpu; /* vcpu timer interrupt delivers to */
["xsa261-4.9.patch" (application/octet-stream)]
From: Xen Project Security Team <security@xenproject.org>
Subject: x86/vpt: add support for IO-APIC routed interrupts
And modify the HPET code to make use of it. Currently HPET interrupts
are always treated as ISA and thus injected through the vPIC. This is
wrong because HPET interrupts when not in legacy mode should be
injected from the IO-APIC.
To make things worse, the supported interrupt routing values are set
to [20..23], which clearly falls outside of the ISA range, thus
leading to an ASSERT in debug builds or memory corruption in non-debug
builds because the interrupt injection code will write out of the
bounds of the arch.hvm_domain.vpic array.
Since the HPET interrupt source can change between ISA and IO-APIC
always destroy the timer before changing the mode, or else Xen risks
changing it while the timer is active.
Note that vpt interrupt injection is racy in the sense that the
vIO-APIC RTE entry can be written by the guest in between the call to
pt_irq_masked and hvm_ioapic_assert, or the call to pt_update_irq and
pt_intr_post. Those are not deemed to be security issues, but rather
quirks of the current implementation. In the worse case the guest
might lose interrupts or get multiple interrupt vectors injected for
the same timer source.
This is part of XSA-261.
Address actual and potential compiler warnings. Fix formatting.
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/hpet.c
+++ b/xen/arch/x86/hvm/hpet.c
@@ -264,13 +264,20 @@ static void hpet_set_timer(HPETState *h,
diff = (timer_is_32bit(h, tn) && (-diff > HPET_TINY_TIME_SPAN))
? (uint32_t)diff : 0;
+ destroy_periodic_time(&h->pt[tn]);
if ( (tn <= 1) && (h->hpet.config & HPET_CFG_LEGACY) )
+ {
/* if LegacyReplacementRoute bit is set, HPET specification requires
timer0 be routed to IRQ0 in NON-APIC or IRQ2 in the I/O APIC,
timer1 be routed to IRQ8 in NON-APIC or IRQ8 in the I/O APIC. */
irq = (tn == 0) ? 0 : 8;
+ h->pt[tn].source = PTSRC_isa;
+ }
else
+ {
irq = timer_int_route(h, tn);
+ h->pt[tn].source = PTSRC_ioapic;
+ }
/*
* diff is the time from now when the timer should fire, for a periodic
--- a/xen/arch/x86/hvm/irq.c
+++ b/xen/arch/x86/hvm/irq.c
@@ -41,6 +41,29 @@ static void assert_gsi(struct domain *d,
vioapic_irq_positive_edge(d, ioapic_gsi);
}
+int hvm_ioapic_assert(struct domain *d, unsigned int gsi, bool level)
+{
+ struct hvm_irq *hvm_irq = hvm_domain_irq(d);
+ const struct hvm_vioapic *vioapic;
+ unsigned int pin;
+ int vector;
+
+ if ( gsi >= hvm_irq->nr_gsis )
+ {
+ ASSERT_UNREACHABLE();
+ return -1;
+ }
+
+ spin_lock(&d->arch.hvm_domain.irq_lock);
+ if ( !level || hvm_irq->gsi_assert_count[gsi]++ == 0 )
+ assert_gsi(d, gsi);
+ vioapic = gsi_vioapic(d, gsi, &pin);
+ vector = vioapic ? vioapic->redirtbl[pin].fields.vector : -1;
+ spin_unlock(&d->arch.hvm_domain.irq_lock);
+
+ return vector;
+}
+
static void assert_irq(struct domain *d, unsigned ioapic_gsi, unsigned pic_irq)
{
assert_gsi(d, ioapic_gsi);
--- a/xen/arch/x86/hvm/vpt.c
+++ b/xen/arch/x86/hvm/vpt.c
@@ -107,31 +107,50 @@ static int pt_irq_vector(struct periodic
static int pt_irq_masked(struct periodic_time *pt)
{
struct vcpu *v = pt->vcpu;
- unsigned int gsi, isa_irq, pin;
- struct hvm_vioapic *vioapic;
- uint8_t pic_imr;
+ unsigned int gsi = pt->irq;
- if ( pt->source == PTSRC_lapic )
+ switch ( pt->source )
+ {
+ case PTSRC_lapic:
{
struct vlapic *vlapic = vcpu_vlapic(v);
+
return (!vlapic_enabled(vlapic) ||
(vlapic_get_reg(vlapic, APIC_LVTT) & APIC_LVT_MASKED));
}
- isa_irq = pt->irq;
- gsi = hvm_isa_irq_to_gsi(isa_irq);
- pic_imr = v->domain->arch.hvm_domain.vpic[isa_irq >> 3].imr;
- vioapic = gsi_vioapic(v->domain, gsi, &pin);
- if ( !vioapic )
- {
- dprintk(XENLOG_WARNING, "d%u: invalid GSI (%u) for platform timer\n",
- v->domain->domain_id, gsi);
- domain_crash(v->domain);
- return -1;
+ case PTSRC_isa:
+ {
+ uint8_t pic_imr = v->domain->arch.hvm_domain.vpic[pt->irq >> 3].imr;
+
+ /* Check if the interrupt is unmasked in the PIC. */
+ if ( !(pic_imr & (1 << (pt->irq & 7))) && vlapic_accept_pic_intr(v) )
+ return 0;
+
+ gsi = hvm_isa_irq_to_gsi(pt->irq);
+ }
+
+ /* Fallthrough to check if the interrupt is masked on the IO APIC. */
+ case PTSRC_ioapic:
+ {
+ unsigned int pin;
+ const struct hvm_vioapic *vioapic = gsi_vioapic(v->domain, gsi, &pin);
+
+ if ( !vioapic )
+ {
+ dprintk(XENLOG_WARNING,
+ "d%d: invalid GSI (%u) for platform timer\n",
+ v->domain->domain_id, gsi);
+ domain_crash(v->domain);
+ return -1;
+ }
+
+ return vioapic->redirtbl[pin].fields.mask;
+ }
}
- return (((pic_imr & (1 << (isa_irq & 7))) || !vlapic_accept_pic_intr(v)) &&
- vioapic->redirtbl[pin].fields.mask);
+ ASSERT_UNREACHABLE();
+ return 1;
}
static void pt_lock(struct periodic_time *pt)
@@ -252,7 +271,7 @@ int pt_update_irq(struct vcpu *v)
struct list_head *head = &v->arch.hvm_vcpu.tm_list;
struct periodic_time *pt, *temp, *earliest_pt;
uint64_t max_lag;
- int irq, is_lapic;
+ int irq, pt_vector = -1;
spin_lock(&v->arch.hvm_vcpu.tm_lock);
@@ -288,29 +307,42 @@ int pt_update_irq(struct vcpu *v)
earliest_pt->irq_issued = 1;
irq = earliest_pt->irq;
- is_lapic = (earliest_pt->source == PTSRC_lapic);
spin_unlock(&v->arch.hvm_vcpu.tm_lock);
- if ( is_lapic )
- vlapic_set_irq(vcpu_vlapic(v), irq, 0);
- else
+ switch ( earliest_pt->source )
{
+ case PTSRC_lapic:
+ /*
+ * If periodic timer interrupt is handled by lapic, its vector in
+ * IRR is returned and used to set eoi_exit_bitmap for virtual
+ * interrupt delivery case. Otherwise return -1 to do nothing.
+ */
+ vlapic_set_irq(vcpu_vlapic(v), irq, 0);
+ pt_vector = irq;
+ break;
+
+ case PTSRC_isa:
hvm_isa_irq_deassert(v->domain, irq);
hvm_isa_irq_assert(v->domain, irq);
+
+ if ( platform_legacy_irq(irq) && vlapic_accept_pic_intr(v) &&
+ v->domain->arch.hvm_domain.vpic[irq >> 3].int_output )
+ return -1;
+
+ pt_vector = pt_irq_vector(earliest_pt, hvm_intsrc_lapic);
+ break;
+
+ case PTSRC_ioapic:
+ /*
+ * NB: At the moment IO-APIC routed interrupts generated by vpt devices
+ * (HPET) are edge-triggered.
+ */
+ pt_vector = hvm_ioapic_assert(v->domain, irq, false);
+ break;
}
- /*
- * If periodic timer interrut is handled by lapic, its vector in
- * IRR is returned and used to set eoi_exit_bitmap for virtual
- * interrupt delivery case. Otherwise return -1 to do nothing.
- */
- if ( !is_lapic &&
- platform_legacy_irq(irq) && vlapic_accept_pic_intr(v) &&
- (&v->domain->arch.hvm_domain)->vpic[irq >> 3].int_output )
- return -1;
- else
- return pt_irq_vector(earliest_pt, hvm_intsrc_lapic);
+ return pt_vector;
}
static struct periodic_time *is_pt_irq(
@@ -405,7 +437,14 @@ void create_periodic_time(
struct vcpu *v, struct periodic_time *pt, uint64_t delta,
uint64_t period, uint8_t irq, time_cb *cb, void *data)
{
- ASSERT(pt->source != 0);
+ if ( !pt->source ||
+ (pt->irq >= NR_ISAIRQS && pt->source == PTSRC_isa) ||
+ (pt->irq >= hvm_domain_irq(v->domain)->nr_gsis &&
+ pt->source == PTSRC_ioapic) )
+ {
+ ASSERT_UNREACHABLE();
+ return;
+ }
destroy_periodic_time(pt);
@@ -485,7 +524,7 @@ static void pt_adjust_vcpu(struct period
{
int on_list;
- ASSERT(pt->source == PTSRC_isa);
+ ASSERT(pt->source == PTSRC_isa || pt->source == PTSRC_ioapic);
if ( pt->vcpu == NULL )
return;
--- a/xen/include/asm-x86/hvm/irq.h
+++ b/xen/include/asm-x86/hvm/irq.h
@@ -106,4 +106,7 @@ struct hvm_intack hvm_vcpu_has_pending_i
struct hvm_intack hvm_vcpu_ack_pending_irq(struct vcpu *v,
struct hvm_intack intack);
+/* Assert an IO APIC pin. */
+int hvm_ioapic_assert(struct domain *d, unsigned int gsi, bool level);
+
#endif /* __ASM_X86_HVM_IRQ_H__ */
--- a/xen/include/asm-x86/hvm/vpt.h
+++ b/xen/include/asm-x86/hvm/vpt.h
@@ -44,6 +44,7 @@ struct periodic_time {
bool_t warned_timeout_too_short;
#define PTSRC_isa 1 /* ISA time source */
#define PTSRC_lapic 2 /* LAPIC time source */
+#define PTSRC_ioapic 3 /* IOAPIC time source */
u8 source; /* PTSRC_ */
u8 irq;
struct vcpu *vcpu; /* vcpu timer interrupt delivers to */
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic