From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH v2 2/5] KVM: x86: Emulator performs code segment checks on read access Date: Mon, 13 Oct 2014 14:31:53 +0300 Message-ID: <20141013113152.GB9340@minantech.com> References: <20141006203238.GA4989@potion.brq.redhat.com> <1412906870-4322-1-git-send-email-namit@cs.technion.ac.il> <20141010155455.GA17902@potion.brq.redhat.com> <5438FAD6.3010805@redhat.com> <543A7042.2050507@redhat.com> <543B0B9F.9060708@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=cp1255 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Paolo Bonzini , Radim =?utf-8?B?S3LEjW3DocWZ?= , Nadav Amit , kvm@vger.kernel.org To: Nadav Amit Return-path: Received: from mail-wg0-f45.google.com ([74.125.82.45]:35928 "EHLO mail-wg0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753090AbaJMLb5 convert rfc822-to-8bit (ORCPT ); Mon, 13 Oct 2014 07:31:57 -0400 Received: by mail-wg0-f45.google.com with SMTP id m15so8537897wgh.16 for ; Mon, 13 Oct 2014 04:31:56 -0700 (PDT) Content-Disposition: inline In-Reply-To: <543B0B9F.9060708@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, Oct 13, 2014 at 02:15:43AM +0300, Nadav Amit wrote: >=20 >=20 > On 10/12/14 3:12 PM, Paolo Bonzini wrote: > > Il 12/10/2014 08:57, Nadav Amit ha scritto: > >> Looks good. I=92ll give it a try but it is hard to give a definiti= ve > >> answer, since the emulator is still bug-ridden. > >=20 > > Yes, we need to write unit tests for this, especially the conformin= g > > case. A bit of a pain to get kvm-unit-tests in ring 3 (access.flat > > does it), but I'll give it a shot. > >=20 > > Paolo > >=20 >=20 > I think the problem might be even more fundamental. > According to the SDM, the privilege level checks (CPL/DPL/RPL) are on= ly performed when the segment is loaded; I see no reference to privileg= e checks when data is accessed. > You should be able to load a segment with DPL=3D0 while you are in CP= L=3D0, then change CPL to 3 and still access the segment (obviously, it= is not the best practice). >=20 > In that case, all the privilege checks in __linearize are redundant a= nd for some extent incorrect. > Obviously, I am afraid to submit a patch that removes them, since if = the privilege checks of __linearize are needed in certain case, this ma= y introduce security problem. >=20 > Do you agree? >=20 3a78a4f46302bfc83602a53dfa4dcbe76a7a1f5f removed RPL check from __linea= rize already, so you are probably right, but better verify it on real HW. -- Gleb.