From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCHv2 4/5] KVM: emulator: move linearize() out of emulator code. Date: Mon, 25 Jun 2012 17:55:01 +0300 Message-ID: <20120625145501.GB2596@redhat.com> References: <1339502487-30049-5-git-send-email-gleb@redhat.com> <4FE71225.2010504@redhat.com> <20120624132710.GW6533@redhat.com> <4FE7188A.2080500@redhat.com> <20120624142753.GX6533@redhat.com> <4FE86046.6090702@redhat.com> <20120625131253.GA16583@redhat.com> <4FE86A53.2060108@redhat.com> <20120625141750.GA2596@redhat.com> <4FE8767F.1020108@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, mtosatti@redhat.com To: Avi Kivity Return-path: Received: from mx1.redhat.com ([209.132.183.28]:16006 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755632Ab2FYOzC (ORCPT ); Mon, 25 Jun 2012 10:55:02 -0400 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q5PEt2oh017581 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 25 Jun 2012 10:55:02 -0400 Content-Disposition: inline In-Reply-To: <4FE8767F.1020108@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: 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.