From f9105888aa71c43e97ea66e723c78098e5a73e2c Mon Sep 17 00:00:00 2001 From: Jan Beulich Date: Tue, 2 May 2017 12:18:35 +0100 Subject: [PATCH] multicall: deal with early exit conditions In particular changes to guest privilege level require the multicall sequence to be aborted, as hypercalls are permitted from kernel mode only. While likely not very useful in a multicall, also properly handle the return value in the HYPERVISOR_iret case (which should be the guest specified value). This is XSA-213. Reported-by: Jann Horn Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper Acked-by: Julien Grall Gbp-Pq: Name multicall-deal-with-early-exit-condition --- xen/arch/arm/traps.c | 11 +++++++---- xen/arch/x86/hypercall.c | 26 ++++++++++++++++++-------- xen/common/multicall.c | 17 ++++++++++++++--- xen/include/xen/multicall.h | 6 +++++- 4 files changed, 44 insertions(+), 16 deletions(-) diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 90aba2a6d6..0f3ef1ff08 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -1531,7 +1531,7 @@ static bool_t check_multicall_32bit_clean(struct multicall_entry *multi) return true; } -void arch_do_multicall_call(struct mc_state *state) +enum mc_disposition arch_do_multicall_call(struct mc_state *state) { struct multicall_entry *multi = &state->call; arm_hypercall_fn_t call = NULL; @@ -1539,23 +1539,26 @@ void arch_do_multicall_call(struct mc_state *state) if ( multi->op >= ARRAY_SIZE(arm_hypercall_table) ) { multi->result = -ENOSYS; - return; + return mc_continue; } call = arm_hypercall_table[multi->op].fn; if ( call == NULL ) { multi->result = -ENOSYS; - return; + return mc_continue; } if ( is_32bit_domain(current->domain) && !check_multicall_32bit_clean(multi) ) - return; + return mc_continue; multi->result = call(multi->args[0], multi->args[1], multi->args[2], multi->args[3], multi->args[4]); + + return likely(!psr_mode_is_user(guest_cpu_user_regs())) + ? mc_continue : mc_preempt; } /* diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/hypercall.c index d2b533189a..30230414a6 100644 --- a/xen/arch/x86/hypercall.c +++ b/xen/arch/x86/hypercall.c @@ -255,15 +255,19 @@ void pv_hypercall(struct cpu_user_regs *regs) perfc_incr(hypercalls); } -void arch_do_multicall_call(struct mc_state *state) +enum mc_disposition arch_do_multicall_call(struct mc_state *state) { - if ( !is_pv_32bit_vcpu(current) ) + struct vcpu *curr = current; + unsigned long op; + + if ( !is_pv_32bit_vcpu(curr) ) { struct multicall_entry *call = &state->call; - if ( (call->op < ARRAY_SIZE(pv_hypercall_table)) && - pv_hypercall_table[call->op].native ) - call->result = pv_hypercall_table[call->op].native( + op = call->op; + if ( (op < ARRAY_SIZE(pv_hypercall_table)) && + pv_hypercall_table[op].native ) + call->result = pv_hypercall_table[op].native( call->args[0], call->args[1], call->args[2], call->args[3], call->args[4], call->args[5]); else @@ -274,15 +278,21 @@ void arch_do_multicall_call(struct mc_state *state) { struct compat_multicall_entry *call = &state->compat_call; - if ( (call->op < ARRAY_SIZE(pv_hypercall_table)) && - pv_hypercall_table[call->op].compat ) - call->result = pv_hypercall_table[call->op].compat( + op = call->op; + if ( (op < ARRAY_SIZE(pv_hypercall_table)) && + pv_hypercall_table[op].compat ) + call->result = pv_hypercall_table[op].compat( call->args[0], call->args[1], call->args[2], call->args[3], call->args[4], call->args[5]); else call->result = -ENOSYS; } #endif + + return unlikely(op == __HYPERVISOR_iret) + ? mc_exit + : likely(guest_kernel_mode(curr, guest_cpu_user_regs())) + ? mc_continue : mc_preempt; } /* diff --git a/xen/common/multicall.c b/xen/common/multicall.c index 524c9bf090..5d253761b2 100644 --- a/xen/common/multicall.c +++ b/xen/common/multicall.c @@ -40,6 +40,7 @@ do_multicall( struct mc_state *mcs = ¤t->mc_state; uint32_t i; int rc = 0; + enum mc_disposition disp = mc_continue; if ( unlikely(__test_and_set_bit(_MCSF_in_multicall, &mcs->flags)) ) { @@ -50,7 +51,7 @@ do_multicall( if ( unlikely(!guest_handle_okay(call_list, nr_calls)) ) rc = -EFAULT; - for ( i = 0; !rc && i < nr_calls; i++ ) + for ( i = 0; !rc && disp == mc_continue && i < nr_calls; i++ ) { if ( i && hypercall_preempt_check() ) goto preempted; @@ -63,7 +64,7 @@ do_multicall( trace_multicall_call(&mcs->call); - arch_do_multicall_call(mcs); + disp = arch_do_multicall_call(mcs); #ifndef NDEBUG { @@ -77,7 +78,14 @@ do_multicall( } #endif - if ( unlikely(__copy_field_to_guest(call_list, &mcs->call, result)) ) + if ( unlikely(disp == mc_exit) ) + { + if ( __copy_field_to_guest(call_list, &mcs->call, result) ) + /* nothing, best effort only */; + rc = mcs->call.result; + } + else if ( unlikely(__copy_field_to_guest(call_list, &mcs->call, + result)) ) rc = -EFAULT; else if ( mcs->flags & MCSF_call_preempted ) { @@ -93,6 +101,9 @@ do_multicall( guest_handle_add_offset(call_list, 1); } + if ( unlikely(disp == mc_preempt) && i < nr_calls ) + goto preempted; + perfc_incr(calls_to_multicall); perfc_add(calls_from_multicall, i); mcs->flags = 0; diff --git a/xen/include/xen/multicall.h b/xen/include/xen/multicall.h index fff15ebf2c..75e2cc52fe 100644 --- a/xen/include/xen/multicall.h +++ b/xen/include/xen/multicall.h @@ -24,6 +24,10 @@ struct mc_state { }; }; -void arch_do_multicall_call(struct mc_state *mc); +enum mc_disposition { + mc_continue, + mc_exit, + mc_preempt, +} arch_do_multicall_call(struct mc_state *mc); #endif /* __XEN_MULTICALL_H__ */ -- 2.30.2