From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCHv2 4/5] KVM: emulator: move linearize() out of emulator code. Date: Mon, 25 Jun 2012 18:50:06 +0300 Message-ID: <4FE888AE.80801@redhat.com> References: <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> <20120625145501.GB2596@redhat.com> <4FE87DB7.1000905@redhat.com> <20120625153544.GC2596@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, mtosatti@redhat.com To: Gleb Natapov Return-path: Received: from mx1.redhat.com ([209.132.183.28]:5885 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756191Ab2FYPuM (ORCPT ); Mon, 25 Jun 2012 11:50:12 -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 q5PFoCa8011119 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 25 Jun 2012 11:50:12 -0400 In-Reply-To: <20120625153544.GC2596@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 06/25/2012 06:35 PM, Gleb Natapov wrote: >> >> Agree. Though the security issue is limited; the structure won't be >> uninitialized, it would retain values from the previous call. So it's >> limited to intra-guest vulnerabilities. >> > Yes, that's the kind I mean, not host crash. Intra-guest vulnerabilities > should not be taken lightly. From guest POV they are like buggy CPUs > that allows privilege escalation. It's a smaller disaster; I didn't mean to minimize those issues. > >> > >> >> 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. >> >> An alternative is to use two contexts, the base context only holds ops >> and is the parameter to all the callbacks on the non-state APIs, the >> derived context holds the state: >> >> struct x86_emulation_ctxt { >> struct x86_ops *ops; >> /* state that always needs to be initialized, preferablt none */ >> }; >> >> struct x86_insn_ctxt { >> struct x86_emulation_ctxt em; >> /* instruction state */ >> } >> >> and so we have a compile-time split between users of the state and >> non-users. >> > I do not understand how you will divide current ctxt structure between > those two. > > Where will you put those for instance: interruptibility, have_exception, > perm_ok, only_vendor_specific_insn and how can they not be initialized > before each instruction emulation? x86_emulate_ops::get_interruptibility() x86_emulate_ops::set_interruptibility() x86_emulate_ops::exception() x86_decode_insn(struct x86_insn_ctxt *ctxt, unsigned flags) { ctxt->flags = flags; ctxt->perm_ok = false; } In short, instruction emulation state is only seen by instruction emulation functions, the others don't get to see it. -- error compiling committee.c: too many arguments to function