From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [PATCH v2 2/5] KVM: x86: Emulator performs code segment checks on read access Date: Fri, 10 Oct 2014 17:54:56 +0200 Message-ID: <20141010155455.GA17902@potion.brq.redhat.com> References: <20141006203238.GA4989@potion.brq.redhat.com> <1412906870-4322-1-git-send-email-namit@cs.technion.ac.il> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: pbonzini@redhat.com, kvm@vger.kernel.org To: Nadav Amit Return-path: Received: from mx1.redhat.com ([209.132.183.28]:27386 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751312AbaJJPzD (ORCPT ); Fri, 10 Oct 2014 11:55:03 -0400 Content-Disposition: inline In-Reply-To: <1412906870-4322-1-git-send-email-namit@cs.technion.ac.il> Sender: kvm-owner@vger.kernel.org List-ID: 2014-10-10 05:07+0300, Nadav Amit: > When read access is performed using a readable code segment, the "conforming" > and "non-conforming" checks should not be done. As a result, read using > non-conforming readable code segment fails. > > This is according to Intel SDM 5.6.1 ("Accessing Data in Code Segments"). > > One exception is the case of conforming code segment. The SDM says: "Use a > code-segment override prefix (CS) to read a readable... [it is] valid because > the DPL of the code segment selected by the CS register is the same as the > CPL." This is misleading since CS.DPL may be lower (numerically) than CPL, and > CS would still be accessible. The emulator should avoid privilage level checks > for data reads using CS. Ah, after stripping faulty presumptions, I'm not sure this change is enough ... shouldn't we also skip the check on conforming code segments? Method 2 is always valid because the privilege level of a conforming code segment is effectively the same as the CPL, regardless of its DPL. And we couldn't read it from less privileged modes. > The fix is not to perform the "non-conforming" checks if the access is not a > fetch, and never to perform the checks for CS. > > --- > v1->v2: Privilage level checks are always skipped for CS > > Signed-off-by: Nadav Amit > --- > arch/x86/kvm/emulate.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index a46207a..0fee0a0 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -661,9 +661,9 @@ static int __linearize(struct x86_emulate_ctxt *ctxt, > goto bad; > } > cpl = ctxt->ops->cpl(ctxt); > - if (!(desc.type & 8)) { > - /* data segment */ > - if (cpl > desc.dpl) > + if (!fetch) { > + /* data segment or readable code segment */ > + if (cpl > desc.dpl && addr.seg != VCPU_SREG_CS) > goto bad; > } else if ((desc.type & 8) && !(desc.type & 4)) { > /* nonconforming code segment */ > -- > 1.9.1 >