From patchwork Fri Jul 5 07:45:22 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitrii Kuvaiskii X-Patchwork-Id: 13724494 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D522A1442F0; Fri, 5 Jul 2024 07:54:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.10 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720166052; cv=none; b=hMlwSTomUxgVgh/3093j3MEFNtFt9gCWbmcD01jAAouNm9WOcX9la7Tr4udbQpCAcIhMVeNBKVL9B2Ngk3X8xNY3bxWVbaEnmfhKgVAbfZGtoxTjnm2MeQsNlAKgV/cQ+wSwkAdNFBKP7DgTML6aBPHO+ngxrbaWrkV32Vwin4Y= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720166052; c=relaxed/simple; bh=5OWW7SNa0nNrp39nyTHhqkbfg5MVC6mu3/hAnCEpYZY=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version:Content-Type; b=J+W5I6Lxodby/y4Mcbdrdx25RMb7gcNhN7NnTC/tFxcfJYZX97e3ZAzGkofNNhdLjL+B+433TJCCBxnB4sS9F01YsL/YfhdNXExws0a2L9LFNyUhGwG9Q/kAzp+qQ5/73XJ+xXZnyDG4Q1OrhRdeJsCpXAFI4ycwl8f8m03Hk4A= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=Q9MBxORM; arc=none smtp.client-ip=198.175.65.10 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="Q9MBxORM" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1720166050; x=1751702050; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=5OWW7SNa0nNrp39nyTHhqkbfg5MVC6mu3/hAnCEpYZY=; b=Q9MBxORMw82VN/Lfess9v17JoEj1HXwi+6JCc9HYKtgEAyefa0uWIFMF e7bqoW7AQ4mnd6ET6ZxGjX+lZFk/sTQ2rcfJPgcsT6yh5qOYZ+Xwfrj/e qjGPSYALWjNO1ZP4agXcruHYbjufXWlqcpOpfyrtTKvHjvxFlyOEo1jRh l80T44c0jKZ6L5GSnhKRnYlVYkmIBRAu9S8Nt9HopRHpza2HsgCuDrt+0 xEMv/THzapN7Gu2Gdp9E3HNByUVBpiwMvbI5cR+aA5OCNF+G2ME02KGL8 BGtVatGNxY2z40NyP9WQoKEaj+p4H2LDvZNFH33EeIV7afTz8a6Cig9/y Q==; X-CSE-ConnectionGUID: 64zhLc5bSEy3pt7Zh/ND6Q== X-CSE-MsgGUID: 8+ZXjVL5SWW+tRUKmO4MBA== X-IronPort-AV: E=McAfee;i="6700,10204,11123"; a="34892875" X-IronPort-AV: E=Sophos;i="6.09,184,1716274800"; d="scan'208";a="34892875" Received: from orviesa009.jf.intel.com ([10.64.159.149]) by orvoesa102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jul 2024 00:53:42 -0700 X-CSE-ConnectionGUID: +dTIeSjVRxiOIrG7SjdfyA== X-CSE-MsgGUID: biB6J1XATHeOSubFTzNVuQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,184,1716274800"; d="scan'208";a="46918566" Received: from mehlow-prequal01.jf.intel.com ([10.54.102.156]) by orviesa009-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jul 2024 00:53:43 -0700 From: Dmitrii Kuvaiskii To: dave.hansen@linux.intel.com, jarkko@kernel.org, kai.huang@intel.com, haitao.huang@linux.intel.com, reinette.chatre@intel.com, linux-sgx@vger.kernel.org, linux-kernel@vger.kernel.org Cc: mona.vij@intel.com, kailun.qin@intel.com, stable@vger.kernel.org Subject: [PATCH v4 1/3] x86/sgx: Split SGX_ENCL_PAGE_BEING_RECLAIMED into two flags Date: Fri, 5 Jul 2024 00:45:22 -0700 Message-Id: <20240705074524.443713-2-dmitrii.kuvaiskii@intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240705074524.443713-1-dmitrii.kuvaiskii@intel.com> References: <20240705074524.443713-1-dmitrii.kuvaiskii@intel.com> Precedence: bulk X-Mailing-List: linux-sgx@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Organization: Intel Deutschland GmbH - Registered Address: Am Campeon 10, 85579 Neubiberg, Germany SGX_ENCL_PAGE_BEING_RECLAIMED flag is set when the enclave page is being reclaimed (moved to the backing store). This flag however has two logical meanings: 1. Don't attempt to load the enclave page (the page is busy). 2. Don't attempt to remove the PCMD page corresponding to this enclave page (the PCMD page is busy). To reflect these two meanings, split SGX_ENCL_PAGE_BEING_RECLAIMED into two flags: SGX_ENCL_PAGE_BUSY and SGX_ENCL_PAGE_PCMD_BUSY. Currently, both flags are set only when the enclave page is being reclaimed. A future commit will introduce a new case when the enclave page is being removed; this new case will set only the SGX_ENCL_PAGE_BUSY flag. Cc: stable@vger.kernel.org Signed-off-by: Dmitrii Kuvaiskii Reviewed-by: Haitao Huang Acked-by: Kai Huang --- arch/x86/kernel/cpu/sgx/encl.c | 16 +++++++--------- arch/x86/kernel/cpu/sgx/encl.h | 10 ++++++++-- arch/x86/kernel/cpu/sgx/main.c | 4 ++-- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c index 279148e72459..c0a3c00284c8 100644 --- a/arch/x86/kernel/cpu/sgx/encl.c +++ b/arch/x86/kernel/cpu/sgx/encl.c @@ -46,10 +46,10 @@ static int sgx_encl_lookup_backing(struct sgx_encl *encl, unsigned long page_ind * a check if an enclave page sharing the PCMD page is in the process of being * reclaimed. * - * The reclaimer sets the SGX_ENCL_PAGE_BEING_RECLAIMED flag when it - * intends to reclaim that enclave page - it means that the PCMD page - * associated with that enclave page is about to get some data and thus - * even if the PCMD page is empty, it should not be truncated. + * The reclaimer sets the SGX_ENCL_PAGE_PCMD_BUSY flag when it intends to + * reclaim that enclave page - it means that the PCMD page associated with that + * enclave page is about to get some data and thus even if the PCMD page is + * empty, it should not be truncated. * * Context: Enclave mutex (&sgx_encl->lock) must be held. * Return: 1 if the reclaimer is about to write to the PCMD page @@ -77,8 +77,7 @@ static int reclaimer_writing_to_pcmd(struct sgx_encl *encl, * Stop when reaching the SECS page - it does not * have a page_array entry and its reclaim is * started and completed with enclave mutex held so - * it does not use the SGX_ENCL_PAGE_BEING_RECLAIMED - * flag. + * it does not use the SGX_ENCL_PAGE_PCMD_BUSY flag. */ if (addr == encl->base + encl->size) break; @@ -91,8 +90,7 @@ static int reclaimer_writing_to_pcmd(struct sgx_encl *encl, * VA page slot ID uses same bit as the flag so it is important * to ensure that the page is not already in backing store. */ - if (entry->epc_page && - (entry->desc & SGX_ENCL_PAGE_BEING_RECLAIMED)) { + if (entry->epc_page && (entry->desc & SGX_ENCL_PAGE_PCMD_BUSY)) { reclaimed = 1; break; } @@ -257,7 +255,7 @@ static struct sgx_encl_page *__sgx_encl_load_page(struct sgx_encl *encl, /* Entry successfully located. */ if (entry->epc_page) { - if (entry->desc & SGX_ENCL_PAGE_BEING_RECLAIMED) + if (entry->desc & SGX_ENCL_PAGE_BUSY) return ERR_PTR(-EBUSY); return entry; diff --git a/arch/x86/kernel/cpu/sgx/encl.h b/arch/x86/kernel/cpu/sgx/encl.h index f94ff14c9486..11b09899cd92 100644 --- a/arch/x86/kernel/cpu/sgx/encl.h +++ b/arch/x86/kernel/cpu/sgx/encl.h @@ -22,8 +22,14 @@ /* 'desc' bits holding the offset in the VA (version array) page. */ #define SGX_ENCL_PAGE_VA_OFFSET_MASK GENMASK_ULL(11, 3) -/* 'desc' bit marking that the page is being reclaimed. */ -#define SGX_ENCL_PAGE_BEING_RECLAIMED BIT(3) +/* 'desc' bit indicating that the page is busy (e.g. being reclaimed). */ +#define SGX_ENCL_PAGE_BUSY BIT(2) + +/* + * 'desc' bit indicating that PCMD page associated with the enclave page is + * busy (e.g. because the enclave page is being reclaimed). + */ +#define SGX_ENCL_PAGE_PCMD_BUSY BIT(3) struct sgx_encl_page { unsigned long desc; diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index 166692f2d501..e94b09c43673 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -204,7 +204,7 @@ static void sgx_encl_ewb(struct sgx_epc_page *epc_page, void *va_slot; int ret; - encl_page->desc &= ~SGX_ENCL_PAGE_BEING_RECLAIMED; + encl_page->desc &= ~(SGX_ENCL_PAGE_BUSY | SGX_ENCL_PAGE_PCMD_BUSY); va_page = list_first_entry(&encl->va_pages, struct sgx_va_page, list); @@ -340,7 +340,7 @@ static void sgx_reclaim_pages(void) goto skip; } - encl_page->desc |= SGX_ENCL_PAGE_BEING_RECLAIMED; + encl_page->desc |= SGX_ENCL_PAGE_BUSY | SGX_ENCL_PAGE_PCMD_BUSY; mutex_unlock(&encl_page->encl->lock); continue; From patchwork Fri Jul 5 07:45:23 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Dmitrii Kuvaiskii X-Patchwork-Id: 13724493 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6C5D9132103; Fri, 5 Jul 2024 07:54:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.10 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720166050; cv=none; b=OpQr4JAtUyqgIlKO1iXu2ViqMD1GhU4DWtMtAYwUl4B1HfqiZlcLcrGnx1TG6cbVyaOt5NNTVPl9Sz6sgnzB25BG9wlSkmXDk/3e2aKrtzg/tyuySJruipOXGSHtZ2T1v0I6A0J8HAuD6w9QyGPrxxijo756pcdduLv2gYkYUPg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720166050; c=relaxed/simple; bh=4yA/SqWhaSDPZ6/d0XAruoXDmzVaIS+v2bhSzKsdbBs=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version:Content-Type; b=TRF7xjp5Mz49uFuHhCMAB6cDo8tiCgllUEZItPmh3ZFWU3YsOSspHMNdBOYhWQA81IepwMzNiFgoMKem/TJwu+6Nw17gOqyC00gEpe0vK+ROIfvLjzwBGYzGqs/I+puzapJuHzWedvQpZIyGBw/Qzh8FzA79seXxh3Cp1tt2/HU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=J++aJw33; arc=none smtp.client-ip=198.175.65.10 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="J++aJw33" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1720166048; x=1751702048; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=4yA/SqWhaSDPZ6/d0XAruoXDmzVaIS+v2bhSzKsdbBs=; b=J++aJw33HUZ2bDMDgSnRCNppRa/bX3ZKFMHGrvc9ePdiO5F0Z96ry18B UKZEPQdjbB5uHmNvwZKoMNg8FIkKeZ7rrxk5HAClN7KUR0scOGUAwbbsX brPEQK+N/dVBW9/ZNRYQ7OVRBt7rbzP+wROL4996CvmuYrLAmXoUut20H uzinBGmYdaNNsoPq7RWmRWTHlGnfPCZ+acNxque4s/5iVvMRjY8hUMORs F5uJVwfNz+eab8aVJUqVdPGFssDaYjBVFooP1Z2jmzr5xHvSsplglOXn+ v6etXbnMusK3Nviq6pxPy3WwBjhRabhiLNqJF/Lc/RE3fzgSr+xzWiF0i w==; X-CSE-ConnectionGUID: h/PvsrUiQWavRV6IpuIJ9g== X-CSE-MsgGUID: TxdkP8OgS22YtMj+5ZdXmA== X-IronPort-AV: E=McAfee;i="6700,10204,11123"; a="34892877" X-IronPort-AV: E=Sophos;i="6.09,184,1716274800"; d="scan'208";a="34892877" Received: from orviesa009.jf.intel.com ([10.64.159.149]) by orvoesa102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jul 2024 00:53:42 -0700 X-CSE-ConnectionGUID: TX5lsDZVTyW1ptroyuUwbg== X-CSE-MsgGUID: l5NbbIlXTSWBOTPYLj3F0w== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,184,1716274800"; d="scan'208";a="46918570" Received: from mehlow-prequal01.jf.intel.com ([10.54.102.156]) by orviesa009-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jul 2024 00:53:43 -0700 From: Dmitrii Kuvaiskii To: dave.hansen@linux.intel.com, jarkko@kernel.org, kai.huang@intel.com, haitao.huang@linux.intel.com, reinette.chatre@intel.com, linux-sgx@vger.kernel.org, linux-kernel@vger.kernel.org Cc: mona.vij@intel.com, kailun.qin@intel.com, stable@vger.kernel.org, =?utf-8?q?Marcelina_Ko=C5=9Bcielnicka?= Subject: [PATCH v4 2/3] x86/sgx: Resolve EAUG race where losing thread returns SIGBUS Date: Fri, 5 Jul 2024 00:45:23 -0700 Message-Id: <20240705074524.443713-3-dmitrii.kuvaiskii@intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240705074524.443713-1-dmitrii.kuvaiskii@intel.com> References: <20240705074524.443713-1-dmitrii.kuvaiskii@intel.com> Precedence: bulk X-Mailing-List: linux-sgx@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Organization: Intel Deutschland GmbH - Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Imagine an mmap()'d file. Two threads touch the same address at the same time and fault. Both allocate a physical page and race to install a PTE for that page. Only one will win the race. The loser frees its page, but still continues handling the fault as a success and returns VM_FAULT_NOPAGE from the fault handler. The same race can happen with SGX. But there's a bug: the loser in the SGX steers into a failure path. The loser EREMOVE's the winner's EPC page, then returns SIGBUS, likely killing the app. Fix the SGX loser's behavior. Change the return code to VM_FAULT_NOPAGE to avoid SIGBUS and call sgx_free_epc_page() which avoids EREMOVE'ing the winner's page and only frees the page that the loser allocated. The race can be illustrated as follows: /* /* * Fault on CPU1 * Fault on CPU2 * on enclave page X * on enclave page X */ */ sgx_vma_fault() { sgx_vma_fault() { xa_load(&encl->page_array) xa_load(&encl->page_array) == NULL --> == NULL --> sgx_encl_eaug_page() { sgx_encl_eaug_page() { ... ... /* /* * alloc encl_page * alloc encl_page */ */ mutex_lock(&encl->lock); /* * alloc EPC page */ epc_page = sgx_alloc_epc_page(...); /* * add page to enclave's xarray */ xa_insert(&encl->page_array, ...); /* * add page to enclave via EAUG * (page is in pending state) */ /* * add PTE entry */ vmf_insert_pfn(...); mutex_unlock(&encl->lock); return VM_FAULT_NOPAGE; } } /* * All good up to here: enclave page * successfully added to enclave, * ready for EACCEPT from user space */ mutex_lock(&encl->lock); /* * alloc EPC page */ epc_page = sgx_alloc_epc_page(...); /* * add page to enclave's xarray, * this fails with -EBUSY as this * page was already added by CPU2 */ xa_insert(&encl->page_array, ...); err_out_shrink: sgx_encl_free_epc_page(epc_page) { /* * remove page via EREMOVE * * *BUG*: page added by CPU2 is * yanked from enclave while it * remains accessible from OS * perspective (PTE installed) */ /* * free EPC page */ sgx_free_epc_page(epc_page); } mutex_unlock(&encl->lock); /* * *BUG*: SIGBUS is returned * for a valid enclave page */ return VM_FAULT_SIGBUS; } } Fixes: 5a90d2c3f5ef ("x86/sgx: Support adding of pages to an initialized enclave") Cc: stable@vger.kernel.org Reported-by: Marcelina Kościelnicka Suggested-by: Reinette Chatre Signed-off-by: Dmitrii Kuvaiskii Reviewed-by: Haitao Huang Reviewed-by: Jarkko Sakkinen Reviewed-by: Reinette Chatre --- arch/x86/kernel/cpu/sgx/encl.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c index c0a3c00284c8..9f7f9e57cdeb 100644 --- a/arch/x86/kernel/cpu/sgx/encl.c +++ b/arch/x86/kernel/cpu/sgx/encl.c @@ -380,8 +380,11 @@ static vm_fault_t sgx_encl_eaug_page(struct vm_area_struct *vma, * If ret == -EBUSY then page was created in another flow while * running without encl->lock */ - if (ret) + if (ret) { + if (ret == -EBUSY) + vmret = VM_FAULT_NOPAGE; goto err_out_shrink; + } pginfo.secs = (unsigned long)sgx_get_epc_virt_addr(encl->secs.epc_page); pginfo.addr = encl_page->desc & PAGE_MASK; @@ -417,7 +420,7 @@ static vm_fault_t sgx_encl_eaug_page(struct vm_area_struct *vma, err_out_shrink: sgx_encl_shrink(encl, va_page); err_out_epc: - sgx_encl_free_epc_page(epc_page); + sgx_free_epc_page(epc_page); err_out_unlock: mutex_unlock(&encl->lock); kfree(encl_page); From patchwork Fri Jul 5 07:45:24 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitrii Kuvaiskii X-Patchwork-Id: 13724495 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 35013144D01; Fri, 5 Jul 2024 07:54:10 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.10 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720166052; cv=none; b=GRVA6HE7re/0pzV0NeQagF4l6p8PlbavD+Z554uj9PT5OoQ6iz9pjzVlIwDW9+Rw52fVP/uz1N3GALedV9S63HPiyrwgcNEyqld82+wcuP47l6HMSbSlJ5yDUKtmWQTzJPHR4HSAGZQQnCf4Nf3fsz/97KSFhfNE5XMQoJFXDb4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1720166052; c=relaxed/simple; bh=/HURQ9P7Vc7T3PAxMwL3oaZWdqdQeeykS7R4xZmGSKg=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version:Content-Type; b=gRKoa85iP7L0CK3X09FLAsMB2/GgjQW12/65rWv9ZpOdYNxdi9nIDT+kN9RTbB4kzTK30BLIOVwHiJ318NccikEboR44ZwJ5QvNLZ532xBN6JhBinDWX13iTuY1eYbV+zrEaTgdaAbkO9ZzThTqm9XB6FvX/GFCvbsS2ED1aLJ8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com; spf=pass smtp.mailfrom=intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=jiXSWns7; arc=none smtp.client-ip=198.175.65.10 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="jiXSWns7" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1720166051; x=1751702051; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=/HURQ9P7Vc7T3PAxMwL3oaZWdqdQeeykS7R4xZmGSKg=; b=jiXSWns7L2hZeZ+UCUofvfygaMq5xz62X+/imgeYXWd7kCUWoSw/ZqsZ cyPQwIjdSu1dI1rpkoh7FqqG4HSKwV611CLH37L4qK27In+aDbeakdBGC cyMsczNHEf/4hDa5isQRtJmMLz33DrEbtfhCtIpHnTU0OL5KNZF/0ksJe pOUZH+dpXTO/JyBF3Ro/tekUPVvmVXgWfW9R9ttQr0uIhi6EFT1BwoVc6 XhNNS66s7Xj4RuWL7RauyyYx2opcEZkYSMliSAd0dls5Or34RuK1rzb7P mkRu2Iwlczb4o3jpHJr0M7kK0Q3aF8WbUHdVzLPmSwXyIV7tGwuMsxBcA w==; X-CSE-ConnectionGUID: LkWUS+2jTHKq0ccP4Q5d6A== X-CSE-MsgGUID: i+r4djDjR1+vk+S0AnbKzQ== X-IronPort-AV: E=McAfee;i="6700,10204,11123"; a="34892880" X-IronPort-AV: E=Sophos;i="6.09,184,1716274800"; d="scan'208";a="34892880" Received: from orviesa009.jf.intel.com ([10.64.159.149]) by orvoesa102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jul 2024 00:53:42 -0700 X-CSE-ConnectionGUID: tZahdZq+T3Oz49DbNqUNHw== X-CSE-MsgGUID: +UmtMMtrRa+qoAGmJw7l3g== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.09,184,1716274800"; d="scan'208";a="46918572" Received: from mehlow-prequal01.jf.intel.com ([10.54.102.156]) by orviesa009-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Jul 2024 00:53:43 -0700 From: Dmitrii Kuvaiskii To: dave.hansen@linux.intel.com, jarkko@kernel.org, kai.huang@intel.com, haitao.huang@linux.intel.com, reinette.chatre@intel.com, linux-sgx@vger.kernel.org, linux-kernel@vger.kernel.org Cc: mona.vij@intel.com, kailun.qin@intel.com, stable@vger.kernel.org Subject: [PATCH v4 3/3] x86/sgx: Resolve EREMOVE page vs EAUG page data race Date: Fri, 5 Jul 2024 00:45:24 -0700 Message-Id: <20240705074524.443713-4-dmitrii.kuvaiskii@intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20240705074524.443713-1-dmitrii.kuvaiskii@intel.com> References: <20240705074524.443713-1-dmitrii.kuvaiskii@intel.com> Precedence: bulk X-Mailing-List: linux-sgx@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Organization: Intel Deutschland GmbH - Registered Address: Am Campeon 10, 85579 Neubiberg, Germany Two enclave threads may try to add and remove the same enclave page simultaneously (e.g., if the SGX runtime supports both lazy allocation and MADV_DONTNEED semantics). Consider some enclave page added to the enclave. User space decides to temporarily remove this page (e.g., emulating the MADV_DONTNEED semantics) on CPU1. At the same time, user space performs a memory access on the same page on CPU2, which results in a #PF and ultimately in sgx_vma_fault(). Scenario proceeds as follows: /* * CPU1: User space performs * ioctl(SGX_IOC_ENCLAVE_REMOVE_PAGES) * on enclave page X */ sgx_encl_remove_pages() { mutex_lock(&encl->lock); entry = sgx_encl_load_page(encl); /* * verify that page is * trimmed and accepted */ mutex_unlock(&encl->lock); /* * remove PTE entry; cannot * be performed under lock */ sgx_zap_enclave_ptes(encl); /* * Fault on CPU2 on same page X */ sgx_vma_fault() { /* * PTE entry was removed, but the * page is still in enclave's xarray */ xa_load(&encl->page_array) != NULL -> /* * SGX driver thinks that this page * was swapped out and loads it */ mutex_lock(&encl->lock); /* * this is effectively a no-op */ entry = sgx_encl_load_page_in_vma(); /* * add PTE entry * * *BUG*: a PTE is installed for a * page in process of being removed */ vmf_insert_pfn(...); mutex_unlock(&encl->lock); return VM_FAULT_NOPAGE; } /* * continue with page removal */ mutex_lock(&encl->lock); sgx_encl_free_epc_page(epc_page) { /* * remove page via EREMOVE */ /* * free EPC page */ sgx_free_epc_page(epc_page); } xa_erase(&encl->page_array); mutex_unlock(&encl->lock); } Here, CPU1 removed the page. However CPU2 installed the PTE entry on the same page. This enclave page becomes perpetually inaccessible (until another SGX_IOC_ENCLAVE_REMOVE_PAGES ioctl). This is because the page is marked accessible in the PTE entry but is not EAUGed, and any subsequent access to this page raises a fault: with the kernel believing there to be a valid VMA, the unlikely error code X86_PF_SGX encountered by code path do_user_addr_fault() -> access_error() causes the SGX driver's sgx_vma_fault() to be skipped and user space receives a SIGSEGV instead. The userspace SIGSEGV handler cannot perform EACCEPT because the page was not EAUGed. Thus, the user space is stuck with the inaccessible page. Fix this race by forcing the fault handler on CPU2 to back off if the page is currently being removed (on CPU1). This is achieved by setting SGX_ENCL_PAGE_BUSY flag right-before the first mutex_unlock() in sgx_encl_remove_pages(). Upon loading the page, CPU2 checks whether this page is busy, and if yes then CPU2 backs off and waits until the page is completely removed. After that, any memory access to this page results in a normal "allocate and EAUG a page on #PF" flow. Fixes: 9849bb27152c ("x86/sgx: Support complete page removal") Cc: stable@vger.kernel.org Signed-off-by: Dmitrii Kuvaiskii Reviewed-by: Haitao Huang --- arch/x86/kernel/cpu/sgx/ioctl.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index 5d390df21440..02441883401d 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -1141,7 +1141,14 @@ static long sgx_encl_remove_pages(struct sgx_encl *encl, /* * Do not keep encl->lock because of dependency on * mmap_lock acquired in sgx_zap_enclave_ptes(). + * + * Releasing encl->lock leads to a data race: while CPU1 + * performs sgx_zap_enclave_ptes() and removes the PTE entry + * for the enclave page, CPU2 may attempt to load this page + * (because the page is still in enclave's xarray). To prevent + * CPU2 from loading the page, mark the page as busy. */ + entry->desc |= SGX_ENCL_PAGE_BUSY; mutex_unlock(&encl->lock); sgx_zap_enclave_ptes(encl, addr);