File 5af1daa9-x86-HVM-guard-against-bogus-emulator-ioreq-state.patch of Package xen.10697
# Commit 92938e5d149669033aecdfb3d1396948d49d1887
# Date 2018-05-08 18:13:13 +0100
# Author Jan Beulich <jbeulich@suse.com>
# Committer Andrew Cooper <andrew.cooper3@citrix.com>
x86/HVM: guard against emulator driving ioreq state in weird ways
In the case where hvm_wait_for_io() calls wait_on_xen_event_channel(),
p->state ends up being read twice in succession: once to determine that
state != p->state, and then again at the top of the loop. This gives a
compromised emulator a chance to change the state back between the two
reads, potentially keeping Xen in a loop indefinitely.
Instead:
* Read p->state once in each of the wait_on_xen_event_channel() tests,
* re-use that value the next time around,
* and insist that the states continue to transition "forward" (with the
exception of the transition to STATE_IOREQ_NONE).
This is XSA-262.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: George Dunlap <george.dunlap@citrix.com>
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -363,7 +363,7 @@ void hvm_migrate_pirqs(struct vcpu *v)
void hvm_do_resume(struct vcpu *v)
{
ioreq_t *p;
- unsigned int state;
+ unsigned int state, prev_state = STATE_IOREQ_NONE;
check_wakeup_from_wait();
@@ -374,18 +374,32 @@ void hvm_do_resume(struct vcpu *v)
if ( !(p = get_ioreq(v)) )
goto check_inject_trap;
- while ( (state = p->state) != STATE_IOREQ_NONE )
+ state = p->state;
+
+ smp_rmb();
+
+ while ( state != STATE_IOREQ_NONE )
{
- rmb();
- switch ( state )
+ if ( unlikely(state < prev_state) )
+ {
+ gdprintk(XENLOG_ERR, "Weird HVM ioreq state transition %u -> %u\n",
+ prev_state, state);
+ domain_crash(v->domain);
+ return; /* bail */
+ }
+
+ switch ( prev_state = state )
{
case STATE_IORESP_READY: /* IORESP_READY -> NONE */
hvm_io_assist(p);
+ state = STATE_IOREQ_NONE;
break;
case STATE_IOREQ_READY: /* IOREQ_{READY,INPROCESS} -> IORESP_READY */
case STATE_IOREQ_INPROCESS:
wait_on_xen_event_channel(v->arch.hvm_vcpu.xen_port,
- p->state != state);
+ ({ state = p->state;
+ smp_rmb();
+ state != prev_state; }));
break;
default:
gdprintk(XENLOG_ERR, "Weird HVM iorequest state %u\n", state);