From patchwork Mon Mar 30 18:54:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jarkko Sakkinen X-Patchwork-Id: 11466239 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 5E0021667 for ; Mon, 30 Mar 2020 18:55:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 32D6F20771 for ; Mon, 30 Mar 2020 18:55:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726518AbgC3SzC (ORCPT ); Mon, 30 Mar 2020 14:55:02 -0400 Received: from mga09.intel.com ([134.134.136.24]:10574 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726385AbgC3SzC (ORCPT ); Mon, 30 Mar 2020 14:55:02 -0400 IronPort-SDR: eLAE570qSokNKre6n2UE7JNyD6jWYtgDwXAtMWxhuY43pXw5HYMdhce+/XJGdkTdVKDEsPNd3E +52NqqVre1vQ== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Mar 2020 11:55:02 -0700 IronPort-SDR: 34euZLBwgxUsTgbhh2UsuTb0fEXtkTa9Ca9KtEXzfNgzXGf2KzzYLlv+lWrGTkML2GDGv0DdsH OgZDj0Do1lyQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,325,1580803200"; d="scan'208";a="395223054" Received: from plachner-mobl.ger.corp.intel.com (HELO localhost) ([10.252.35.129]) by orsmga004.jf.intel.com with ESMTP; 30 Mar 2020 11:54:59 -0700 From: Jarkko Sakkinen To: linux-sgx@vger.kernel.org Cc: Jarkko Sakkinen , luto@kernel.org, Haitao Huang , Sean Christopherson Subject: [PATCH] x86/sgx: Remove PROT_NONE branch from sgx_encl_may_map(). Date: Mon, 30 Mar 2020 21:54:52 +0300 Message-Id: <20200330185452.1382455-1-jarkko.sakkinen@linux.intel.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Sender: linux-sgx-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-sgx@vger.kernel.org sgx_encl_may_map() always succeeding when PROT_NONE is given is not that useful behaviour as one can just well as do an anonymous mapping as demonstrated by the change in this patch to the test program. As a consequence, remove the special case. Pratically any possible way to make sure that you don't overwrite anything useful in the memory, should be fine. MAP_FIXED does not care what's underneath (if you want't it to care you ought to use MAP_FIXED_NO_REPLACE). After this change, the selftest run called sgx_mmap() only three times (TCS, text, data) instead of four. test_sgx-1811 [002] .... 586.907585: sgx_mmap <-mmap_region test_sgx-1811 [002] .... 586.911752: sgx_mmap <-mmap_region test_sgx-1811 [002] .... 586.911756: sgx_mmap <-mmap_region This also gives more angles to segregate enclave building and mapping as the mmap()'s need to be applied only when the enclave is fully built: Cc: luto@kernel.org Cc: Haitao Huang Cc: Sean Christopherson Signed-off-by: Jarkko Sakkinen --- Documentation/x86/sgx.rst | 13 +++++-------- arch/x86/kernel/cpu/sgx/encl.c | 7 +------ arch/x86/kernel/cpu/sgx/ioctl.c | 5 ----- tools/testing/selftests/sgx/load.c | 3 ++- 4 files changed, 8 insertions(+), 20 deletions(-) diff --git a/Documentation/x86/sgx.rst b/Documentation/x86/sgx.rst index 79de1f01aa5b..9609a3409ad1 100644 --- a/Documentation/x86/sgx.rst +++ b/Documentation/x86/sgx.rst @@ -168,14 +168,11 @@ accounted to the processes of which behalf the kernel happened to be acting on. Access control ============== -`mmap()` permissions are capped by the enclave permissions. The consequences of -this is are: - -1. Pages for a VMA must built before `mmap()` can be applied with the - permissions required for running the enclave. -2. An address range for an enclave can be reserved with a `PROT_NONE` mapping - (not required but usually makes sense to guarantee that the used address - range is available). +`mmap()` permissions are capped by the enclave permissions. A direct +consequence of this is that all the pages for an address range must be added +before `mmap()` can be applied. Effectively an enclave page with minimum +permission in the address range sets the permission cap for the mapping +operation. Usage Models ============ diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c index d6a19bdd1921..e0124a2f22d5 100644 --- a/arch/x86/kernel/cpu/sgx/encl.c +++ b/arch/x86/kernel/cpu/sgx/encl.c @@ -288,8 +288,7 @@ static unsigned int sgx_vma_fault(struct vm_fault *vmf) * * Iterate through the enclave pages contained within [@start, @end) to verify * the permissions requested by @vm_prot_bits do not exceed that of any enclave - * page to be mapped. Page addresses that do not have an associated enclave - * page are interpreted to zero permissions. + * page to be mapped. * * Return: * 0 on success, @@ -308,10 +307,6 @@ int sgx_encl_may_map(struct sgx_encl *encl, unsigned long start, if (!!(current->personality & READ_IMPLIES_EXEC)) return -EACCES; - /* PROT_NONE always succeeds. */ - if (!vm_prot_bits) - return 0; - idx_start = PFN_DOWN(start); idx_end = PFN_DOWN(end - 1); diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 12e1496f8a8b..3af0596530a8 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -481,15 +481,10 @@ static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long src, * * 1. A regular page: PROT_R, PROT_W and PROT_X match the SECINFO permissions. * 2. A TCS page: PROT_R | PROT_W. - * 3. No page: PROT_NONE. * * mmap() is not allowed to surpass the minimum of the maximum protection bits * within the given address range. * - * As stated above, a non-existent page is interpreted as a page with no - * permissions. In effect, this allows mmap() with PROT_NONE to be used to seek - * an address range for the enclave that can be then populated into SECS. - * * If ENCLS opcode fails, that effectively means that EPC has been invalidated. * When this happens the enclave is destroyed and -EIO is returned to the * caller. diff --git a/tools/testing/selftests/sgx/load.c b/tools/testing/selftests/sgx/load.c index 35a2d7a47dd5..53c565347e9e 100644 --- a/tools/testing/selftests/sgx/load.c +++ b/tools/testing/selftests/sgx/load.c @@ -231,7 +231,8 @@ static bool encl_map_area(struct encl *encl) size_t encl_size = encl->encl_size; void *area; - area = mmap(NULL, encl_size * 2, PROT_NONE, MAP_SHARED, encl->fd, 0); + area = mmap(NULL, encl_size * 2, PROT_NONE, + MAP_SHARED | MAP_ANONYMOUS, -1, 0); if (area == MAP_FAILED) { perror("mmap"); return false;