On Mon, Jun 25, 2012 at 05:32:31PM +0300, Avi Kivity wrote: > On 06/25/2012 05:17 PM, Gleb Natapov wrote: > > On Mon, Jun 25, 2012 at 04:40:35PM +0300, Avi Kivity wrote: > >> On 06/25/2012 04:12 PM, Gleb Natapov wrote: > >> > >> >> Right. But I think we can have x86_linearize() that doesn't take a > >> >> context parameter, only ops. > >> >> > >> > All ops take context parameter though. > >> > > >> > >> context is meaningful for: > >> - saving state between executions (decode/execute/execute) > >> - passing state that is not provided via callbacks (regs/mode/flags) > >> - returning results > >> > >> Only the second is relevant, and we're trying to get rid of that too. > >> > > Callbacks were passed pointer to vcpu, but they were changed to get ctxt > > to better encapsulate emulator.c from rest of the KVM. Are you suggesting > > this was a mistake and we need to rework callbacks to receive pointer > > to vcpu again? I hope not :) > > Ouch. I guess we have to pass the context, but not initialize any of it > except ops. That's hacky and error pron. We need to audit that linearize() and all callbacks/functions it uses do not rely on un-initialized state, which is doable now, but who will remember to check that in the future, while changing seemingly unrelated code, which, by a coincidence, called during linearize()? Instant security vulnerability. For security (if not sanity) sake we should really make sure that ctxt is initialized while we are in emulator.c and make as many checks for it as possible. > Later we can extend x86_decode_insn() and the other > functions to follow the same rule. > What rule? We cannot not initialize a context. You can reduce things that should be initialized to minimum (getting GP registers on demand, etc), but still some initialization is needed since ctxt holds emulation state and it needs to be reset before each emulation. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
