From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9D6CAC433E6 for ; Wed, 3 Feb 2021 23:30:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6F77264DE7 for ; Wed, 3 Feb 2021 23:30:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233533AbhBCXaJ (ORCPT ); Wed, 3 Feb 2021 18:30:09 -0500 Received: from mga03.intel.com ([134.134.136.65]:59041 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232990AbhBCXaG (ORCPT ); Wed, 3 Feb 2021 18:30:06 -0500 IronPort-SDR: yI342EANUoR1LHWaRif9FYmVBPlUUQixs88PZT0lJM3XBwwAJnZBJpBPOJGFZMlPVhgKuxj648 QdMECGxj2i2A== X-IronPort-AV: E=McAfee;i="6000,8403,9884"; a="181209889" X-IronPort-AV: E=Sophos;i="5.79,399,1602572400"; d="scan'208";a="181209889" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Feb 2021 15:29:24 -0800 IronPort-SDR: ZCyoeF85HpRQSAqr81wEd8lX9XHoGFN8PP4hIwaoMw0f++/Rh9X86Ek1zwIPcQfrH9TMwkek7J 0jwgstSZ+7jA== X-IronPort-AV: E=Sophos;i="5.79,399,1602572400"; d="scan'208";a="392735178" Received: from rvchebia-mobl.amr.corp.intel.com ([10.251.7.104]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Feb 2021 15:29:20 -0800 Message-ID: Subject: Re: [RFC PATCH v3 23/27] KVM: VMX: Add SGX ENCLS[ECREATE] handler to enforce CPUID restrictions From: Kai Huang To: Sean Christopherson , "Edgecombe, Rick P" Cc: "linux-sgx@vger.kernel.org" , "kvm@vger.kernel.org" , "x86@kernel.org" , "Huang, Haitao" , "luto@kernel.org" , "jarkko@kernel.org" , "Hansen, Dave" , "vkuznets@redhat.com" , "bp@alien8.de" , "mingo@redhat.com" , "tglx@linutronix.de" , "hpa@zytor.com" , "pbonzini@redhat.com" , "joro@8bytes.org" , "wanpengli@tencent.com" , "jmattson@google.com" Date: Thu, 04 Feb 2021 12:29:18 +1300 In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.38.3 (3.38.3-1.fc33) MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-sgx@vger.kernel.org On Wed, 2021-02-03 at 11:36 -0800, Sean Christopherson wrote: > On Wed, Feb 03, 2021, Edgecombe, Rick P wrote: > > On Tue, 2021-01-26 at 22:31 +1300, Kai Huang wrote: > > > +       /* Exit to userspace if copying from a host userspace address > > > fails. */ > > > +       if (sgx_read_hva(vcpu, m_hva, &miscselect, > > > sizeof(miscselect)) || > > > +           sgx_read_hva(vcpu, a_hva, &attributes, > > > sizeof(attributes)) || > > > +           sgx_read_hva(vcpu, x_hva, &xfrm, sizeof(xfrm)) || > > > +           sgx_read_hva(vcpu, s_hva, &size, sizeof(size))) > > > +               return 0; > > > + > > > +       /* Enforce restriction of access to the PROVISIONKEY. */ > > > +       if (!vcpu->kvm->arch.sgx_provisioning_allowed && > > > +           (attributes & SGX_ATTR_PROVISIONKEY)) { > > > +               if (sgx_12_1->eax & SGX_ATTR_PROVISIONKEY) > > > +                       pr_warn_once("KVM: SGX PROVISIONKEY > > > advertised but not allowed\n"); > > > +               kvm_inject_gp(vcpu, 0); > > > +               return 1; > > > +       } > > > + > > > +       /* Enforce CPUID restrictions on MISCSELECT, ATTRIBUTES and > > > XFRM. */ > > > +       if ((u32)miscselect & ~sgx_12_0->ebx || > > > +           (u32)attributes & ~sgx_12_1->eax || > > > +           (u32)(attributes >> 32) & ~sgx_12_1->ebx || > > > +           (u32)xfrm & ~sgx_12_1->ecx || > > > +           (u32)(xfrm >> 32) & ~sgx_12_1->edx) { > > > +               kvm_inject_gp(vcpu, 0); > > > +               return 1; > > > +       } > > > > Don't you need to deep copy the pageinfo.contents struct as well? > > Otherwise the guest could change these after they were checked. > > > > But it seems it is checked by the HW and something is caught that would > > inject a GP anyway? Can you elaborate on the importance of these > > checks? > > Argh, yes. These checks are to allow migration between systems with different > SGX capabilities, and more importantly to prevent userspace from doing an end > around on the restricted access to PROVISIONKEY. > > IIRC, earlier versions did do a deep copy, but then I got clever. Anyways, yeah, > sadly the entire pageinfo.contents page will need to be copied. I don't fully understand the problem. Are you worried about contents being updated by other vcpus during the trap?  And I don't see how copy can avoid this problem. Even you do copy, the content can still be modified afterwards, correct? So what's the point of copying? Looks a better solution is to kick all vcpus and put them into block state while KVM is doing ENCLS for guest.