Re: [PATCH 1/3] KVM: x86 emulator: Fix task switch privilege checks

[Gleb Natapov <gleb@redhat.com>]

On Tue, Jan 24, 2012 at 03:24:50PM +0100, Kevin Wolf wrote:
> Am 24.01.2012 15:16, schrieb Gleb Natapov:
> > On Tue, Jan 24, 2012 at 03:15:13PM +0100, Kevin Wolf wrote:
> >> Am 24.01.2012 15:03, schrieb Joerg Roedel:
> >>> On Mon, Jan 23, 2012 at 05:10:46PM +0100, Kevin Wolf wrote:
> >>>> This patch fixes the problem for VMX. For SVM, the logic used to
> >>>> determine the source of the task switch is buggy, so we can't pass
> >>>> useful information to the emulator there and just disable the check in
> >>>> all cases.
> >>>
> >>> Actually, SVM isn't buggy :) For SVM you do not need to do any
> >>> priviledge checks in software because the hardware already takes care of
> >>> that.
> >>> In other words, KVM only gets a task-switch intercept if the priviledges
> >>> are all checked and correct.
> >>
> >> Okay, that's good to hear. The current code is still buggy because as
> >> Gleb noted it checks against the TSS DPL. We need to disable that check
> >> for SVM then. Also all checks for TASK_SWITCH_GATE indicate that
> >> something is wrong because it will never happen.
> >>
> > Not necessary. Currently all checks for TASK_SWITCH_GATE also check for
> > TASK_SWITCH_CALL, so I think you can fix SVM case in your patch by
> > passing TASK_SWITCH_GATE instead of TASK_SWITCH_CALL to
> > kvm_task_switch().
> 
> Yes, the emulator itself would be fixed by passing TASK_SWITCH_GATE and
> idt_index = -1 (although it looks a bit brittle).
> 
I think it is OK. Emulator should emulate the spec and if workaround is
needed it should be in platform code. By passing TASK_SWITCH_GATE and
idt_index = -1 we do exactly that.

> However, task_switch_interception() itself does some more based on the
> value of reason, for example it decides whether or not to call
> skip_emulated_instruction().
> 
Joerg need to help us here. If intercept of task switch happens before
rip is advanced past instruction that cause it we have to know somehow
that task switch was caused by instruction. It is not enough that HW
checks permission, we still lack essential info.

--
			Gleb.