From patchwork Wed Dec 14 19:21:25 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sean Christopherson X-Patchwork-Id: 9474545 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 5AA6960571 for ; Wed, 14 Dec 2016 19:21:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 43FD928653 for ; Wed, 14 Dec 2016 19:21:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3865628678; Wed, 14 Dec 2016 19:21:28 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from ml01.01.org (ml01.01.org [198.145.21.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id D448028653 for ; Wed, 14 Dec 2016 19:21:27 +0000 (UTC) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 5E9FF81A33 for ; Wed, 14 Dec 2016 11:21:27 -0800 (PST) X-Original-To: intel-sgx-kernel-dev@lists.01.org Delivered-To: intel-sgx-kernel-dev@lists.01.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id B777881A33 for ; Wed, 14 Dec 2016 11:21:26 -0800 (PST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga103.fm.intel.com with ESMTP; 14 Dec 2016 11:21:26 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,347,1477983600"; d="scan'208";a="202621869" Received: from orsmsx104.amr.corp.intel.com ([10.22.225.131]) by fmsmga004.fm.intel.com with ESMTP; 14 Dec 2016 11:21:26 -0800 Received: from orsmsx151.amr.corp.intel.com (10.22.226.38) by ORSMSX104.amr.corp.intel.com (10.22.225.131) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 14 Dec 2016 11:21:25 -0800 Received: from orsmsx108.amr.corp.intel.com ([169.254.2.164]) by ORSMSX151.amr.corp.intel.com ([10.22.226.38]) with mapi id 14.03.0248.002; Wed, 14 Dec 2016 11:21:25 -0800 From: "Christopherson, Sean J" To: 'Jarkko Sakkinen' , "intel-sgx-kernel-dev@lists.01.org" Thread-Topic: [intel-sgx-kernel-dev] [PATCH v8 10/10] intel_sgx: migrate to radix tree for addressing enclave pages Thread-Index: AQHSUVAVyreKjDrw3UKswQbDVhA7xaEH14eg Date: Wed, 14 Dec 2016 19:21:25 +0000 Message-ID: <37306EFA9975BE469F115FDE982C075B9B6A92B2@ORSMSX108.amr.corp.intel.com> References: <20161208123828.21834-1-jarkko.sakkinen@linux.intel.com> <20161208123828.21834-11-jarkko.sakkinen@linux.intel.com> In-Reply-To: <20161208123828.21834-11-jarkko.sakkinen@linux.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNTVkOTEwZWItYmVmNS00N2UwLWJhNzAtYzgyNGVjNzJmNjk1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6ImNtcGRmUEZMMVEzdHZ6Mk1TUjZTVnJqbGlkczI4aHBRaFwvM29yUGx3dG00PSJ9 x-originating-ip: [10.22.254.138] MIME-Version: 1.0 Subject: Re: [intel-sgx-kernel-dev] [PATCH v8 10/10] intel_sgx: migrate to radix tree for addressing enclave pages X-BeenThere: intel-sgx-kernel-dev@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Project: Intel® Software Guard Extensions for Linux*: https://01.org/intel-software-guard-extensions" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-sgx-kernel-dev-bounces@lists.01.org Sender: "intel-sgx-kernel-dev" X-Virus-Scanned: ClamAV using ClamSMTP Jarkko Sakkinen wrote: > @@ -730,6 +705,14 @@ static int __encl_add_page(struct sgx_encl *encl, > goto out; > } > > + ret = radix_tree_insert(&encl->page_tree, > + encl_page->addr >> PAGE_SHIFT, > + &encl_page); > + if (ret) { > + sgx_put_backing(backing, false /* write */); > + goto out; > + } The call to radix_tree_insert is broken, the third parameter should simply be "encl_page", not "&encl_page". This results in a bogus pointer in the tree and manifests as sgx_vma_do_fault failing, usually due entry->epc_page being non-null. > index 5520080..a9ea67f 100644 > --- a/drivers/platform/x86/intel_sgx_vma.c > +++ b/drivers/platform/x86/intel_sgx_vma.c > @@ -171,7 +171,7 @@ static struct sgx_encl_page *sgx_vma_do_fault(struct > vm_area_struct *vma, if (!encl) > return ERR_PTR(-EFAULT); > > - entry = sgx_encl_find_page(encl, addr); > + entry = radix_tree_lookup(&encl->page_tree, addr >> PAGE_SHIFT); > if (!entry) > return ERR_PTR(-EFAULT); The call to radix_tree_lookup needs to be done with encl->lock held. This manifests as "Bus error (Core Dumped)" failures. Suggested fix for the patch: diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c index ab31ba6..e090302 100644 --- a/drivers/platform/x86/intel_sgx_ioctl.c +++ b/drivers/platform/x86/intel_sgx_ioctl.c @@ -707,7 +707,7 @@ static int __encl_add_page(struct sgx_encl *encl, ret = radix_tree_insert(&encl->page_tree, encl_page->addr >> PAGE_SHIFT, - &encl_page); + encl_page); if (ret) { sgx_put_backing(backing, false /* write */); goto out; diff --git a/drivers/platform/x86/intel_sgx_vma.c b/drivers/platform/x86/intel_sgx_vma.c index a9ea67f..cee888d 100644 --- a/drivers/platform/x86/intel_sgx_vma.c +++ b/drivers/platform/x86/intel_sgx_vma.c @@ -171,10 +171,6 @@ static struct sgx_encl_page *sgx_vma_do_fault(struct vm_area_struct *vma, if (!encl) return ERR_PTR(-EFAULT); - entry = radix_tree_lookup(&encl->page_tree, addr >> PAGE_SHIFT); - if (!entry) - return ERR_PTR(-EFAULT); - epc_page = sgx_alloc_page(encl->tgid_ctx, SGX_ALLOC_ATOMIC); if (IS_ERR(epc_page)) /* reinterpret the type as we return an error */ @@ -193,6 +189,12 @@ static struct sgx_encl_page *sgx_vma_do_fault(struct vm_area_struct *vma, goto out; } + entry = radix_tree_lookup(&encl->page_tree, addr >> PAGE_SHIFT); + if (!entry) { + entry = ERR_PTR(-EFAULT); + goto out; + } + if (reserve && (entry->flags & SGX_ENCL_PAGE_RESERVED)) { sgx_dbg(encl, "cannot fault, 0x%p is reserved\n", (void *)entry->addr);