public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	stable@vger.kernel.org, Haitao Huang <haitao.huang@intel.com>,
	Reinette Chatre <reinette.chatre@intel.com>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Jarkko Sakkinen <jarkko@kernel.org>
Subject: [PATCH 5.17 59/75] x86/sgx: Obtain backing storage page with enclave mutex held
Date: Fri,  3 Jun 2022 19:43:43 +0200	[thread overview]
Message-ID: <20220603173823.410844930@linuxfoundation.org> (raw)
In-Reply-To: <20220603173821.749019262@linuxfoundation.org>

From: Reinette Chatre <reinette.chatre@intel.com>

commit 0e4e729a830c1e7f31d3b3fbf8feb355a402b117 upstream.

Haitao reported encountering a WARN triggered by the ENCLS[ELDU]
instruction faulting with a #GP.

The WARN is encountered when the reclaimer evicts a range of
pages from the enclave when the same pages are faulted back
right away.

The SGX backing storage is accessed on two paths: when there
are insufficient free pages in the EPC the reclaimer works
to move enclave pages to the backing storage and as enclaves
access pages that have been moved to the backing storage
they are retrieved from there as part of page fault handling.

An oversubscribed SGX system will often run the reclaimer and
page fault handler concurrently and needs to ensure that the
backing store is accessed safely between the reclaimer and
the page fault handler. This is not the case because the
reclaimer accesses the backing store without the enclave mutex
while the page fault handler accesses the backing store with
the enclave mutex.

Consider the scenario where a page is faulted while a page sharing
a PCMD page with the faulted page is being reclaimed. The
consequence is a race between the reclaimer and page fault
handler, the reclaimer attempting to access a PCMD at the
same time it is truncated by the page fault handler. This
could result in lost PCMD data. Data may still be
lost if the reclaimer wins the race, this is addressed in
the following patch.

The reclaimer accesses pages from the backing storage without
holding the enclave mutex and runs the risk of concurrently
accessing the backing storage with the page fault handler that
does access the backing storage with the enclave mutex held.

In the scenario below a PCMD page is truncated from the backing
store after all its pages have been loaded in to the enclave
at the same time the PCMD page is loaded from the backing store
when one of its pages are reclaimed:

sgx_reclaim_pages() {              sgx_vma_fault() {
                                     ...
                                     mutex_lock(&encl->lock);
                                     ...
                                     __sgx_encl_eldu() {
                                       ...
                                       if (pcmd_page_empty) {
/*
 * EPC page being reclaimed              /*
 * shares a PCMD page with an             * PCMD page truncated
 * enclave page that is being             * while requested from
 * faulted in.                            * reclaimer.
 */                                       */
sgx_encl_get_backing()  <---------->      sgx_encl_truncate_backing_page()
                                        }
                                       mutex_unlock(&encl->lock);
}                                    }

In this scenario there is a race between the reclaimer and the page fault
handler when the reclaimer attempts to get access to the same PCMD page
that is being truncated. This could result in the reclaimer writing to
the PCMD page that is then truncated, causing the PCMD data to be lost,
or in a new PCMD page being allocated. The lost PCMD data may still occur
after protecting the backing store access with the mutex - this is fixed
in the next patch. By ensuring the backing store is accessed with the mutex
held the enclave page state can be made accurate with the
SGX_ENCL_PAGE_BEING_RECLAIMED flag accurately reflecting that a page
is in the process of being reclaimed.

Consistently protect the reclaimer's backing store access with the
enclave's mutex to ensure that it can safely run concurrently with the
page fault handler.

Cc: stable@vger.kernel.org
Fixes: 1728ab54b4be ("x86/sgx: Add a page reclaimer")
Reported-by: Haitao Huang <haitao.huang@intel.com>
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Tested-by: Jarkko Sakkinen <jarkko@kernel.org>
Tested-by: Haitao Huang <haitao.huang@intel.com>
Link: https://lkml.kernel.org/r/fa2e04c561a8555bfe1f4e7adc37d60efc77387b.1652389823.git.reinette.chatre@intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 arch/x86/kernel/cpu/sgx/main.c |    9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -310,6 +310,7 @@ static void sgx_reclaimer_write(struct s
 	sgx_encl_ewb(epc_page, backing);
 	encl_page->epc_page = NULL;
 	encl->secs_child_cnt--;
+	sgx_encl_put_backing(backing);
 
 	if (!encl->secs_child_cnt && test_bit(SGX_ENCL_INITIALIZED, &encl->flags)) {
 		ret = sgx_encl_get_backing(encl, PFN_DOWN(encl->size),
@@ -381,11 +382,14 @@ static void sgx_reclaim_pages(void)
 			goto skip;
 
 		page_index = PFN_DOWN(encl_page->desc - encl_page->encl->base);
+
+		mutex_lock(&encl_page->encl->lock);
 		ret = sgx_encl_get_backing(encl_page->encl, page_index, &backing[i]);
-		if (ret)
+		if (ret) {
+			mutex_unlock(&encl_page->encl->lock);
 			goto skip;
+		}
 
-		mutex_lock(&encl_page->encl->lock);
 		encl_page->desc |= SGX_ENCL_PAGE_BEING_RECLAIMED;
 		mutex_unlock(&encl_page->encl->lock);
 		continue;
@@ -413,7 +417,6 @@ skip:
 
 		encl_page = epc_page->owner;
 		sgx_reclaimer_write(epc_page, &backing[i]);
-		sgx_encl_put_backing(&backing[i]);
 
 		kref_put(&encl_page->encl->refcount, sgx_encl_release);
 		epc_page->flags &= ~SGX_EPC_PAGE_RECLAIMER_TRACKED;



  parent reply	other threads:[~2022-06-03 17:59 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-03 17:42 [PATCH 5.17 00/75] 5.17.13-rc1 review Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.17 01/75] ALSA: usb-audio: Dont get sample rate for MCT Trigger 5 USB-to-HDMI Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.17 02/75] ALSA: hda/realtek: Add quirk for Dell Latitude 7520 Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.17 03/75] ALSA: hda/realtek: Add quirk for the Framework Laptop Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.17 04/75] pinctrl: sunxi: fix f1c100s uart2 function Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.17 05/75] KVM: arm64: Dont hypercall before EL2 init Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.17 06/75] percpu_ref_init(): clean ->percpu_count_ref on failure Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.17 07/75] net: af_key: check encryption module availability consistency Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.17 08/75] nfc: pn533: Fix buggy cleanup order Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.17 09/75] net: ftgmac100: Disable hardware checksum on AST2600 Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.17 10/75] i2c: ismt: Provide a DMA buffer for Interrupt Cause Logging Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.17 11/75] drivers: i2c: thunderx: Allow driver to work with ACPI defined TWSI controllers Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.17 12/75] netfilter: nf_tables: disallow non-stateful expression in sets earlier Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.17 13/75] i2c: ismt: prevent memory corruption in ismt_access() Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.17 14/75] assoc_array: Fix BUG_ON during garbage collect Greg Kroah-Hartman
2022-06-03 17:42 ` [PATCH 5.17 15/75] pipe: make poll_usage boolean and annotate its access Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 16/75] pipe: Fix missing lock in pipe_resize_ring() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 17/75] net: ipa: compute proper aggregation limit Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 18/75] drm/i915: Fix -Wstringop-overflow warning in call to intel_read_wm_latency() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 19/75] exfat: check if cluster num is valid Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 20/75] exfat: fix referencing wrong parent directory information after renaming Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 21/75] netfilter: nft_limit: Clone packet limits cost value Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 22/75] netfilter: nf_tables: sanitize nft_set_desc_concat_parse() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 23/75] netfilter: nf_tables: hold mutex on netns pre_exit path Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 24/75] netfilter: nf_tables: double hook unregistration in netns path Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 25/75] netfilter: conntrack: re-fetch conntrack after insertion Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 26/75] KVM: PPC: Book3S HV: fix incorrect NULL check on list iterator Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 27/75] x86/fpu: KVM: Set the base guest FPU uABI size to sizeof(struct kvm_xsave) Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 28/75] x86/kvm: Alloc dummy async #PF token outside of raw spinlock Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 29/75] x86, kvm: use correct GFP flags for preemption disabled Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 30/75] x86/uaccess: Implement macros for CMPXCHG on user addresses Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 31/75] KVM: x86: Use __try_cmpxchg_user() to update guest PTE A/D bits Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 32/75] KVM: x86: Use __try_cmpxchg_user() to emulate atomic accesses Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 33/75] KVM: x86: fix typo in __try_cmpxchg_user causing non-atomicness Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 34/75] KVM: x86: avoid calling x86 emulator without a decoded instruction Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 35/75] KVM: x86: avoid loading a vCPU after .vm_destroy was called Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 36/75] KVM: x86: Fix the intel_pt PMI handling wrongly considered from guest Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 37/75] KVM: x86: Drop WARNs that assert a triple fault never "escapes" from L2 Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 38/75] KVM: x86/mmu: Dont rebuild page when the page is synced and no tlb flushing is required Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 39/75] KVM: SVM: Use kzalloc for sev ioctl interfaces to prevent kernel data leak Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 40/75] crypto: caam - fix i.MX6SX entropy delay value Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 41/75] crypto: ecrdsa - Fix incorrect use of vli_cmp Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 42/75] zsmalloc: fix races between asynchronous zspage free and page migration Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 43/75] tools/memory-model/README: Update klitmus7 compat table Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 44/75] ALSA: usb-audio: Workaround for clock setup on TEAC devices Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 45/75] ALSA: usb-audio: Add missing ep_idx in fixed EP quirks Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 46/75] ALSA: usb-audio: Configure sync endpoints before data Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 47/75] Bluetooth: hci_qca: Use del_timer_sync() before freeing Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 48/75] ARM: dts: s5pv210: Correct interrupt name for bluetooth in Aries Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 49/75] dm integrity: fix error code in dm_integrity_ctr() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 50/75] dm crypt: make printing of the key constant-time Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 51/75] dm stats: add cond_resched when looping over entries Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 52/75] dm verity: set DM_TARGET_IMMUTABLE feature flag Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 53/75] raid5: introduce MD_BROKEN Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 54/75] fs/ntfs3: validate BOOT sectors_per_clusters Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 55/75] HID: multitouch: Add support for Google Whiskers Touchpad Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 56/75] HID: multitouch: add quirks to enable Lenovo X12 trackpoint Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 57/75] x86/sgx: Disconnect backing page references from dirty status Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 58/75] x86/sgx: Mark PCMD page as dirty when modifying contents Greg Kroah-Hartman
2022-06-03 17:43 ` Greg Kroah-Hartman [this message]
2022-06-03 17:43 ` [PATCH 5.17 60/75] x86/sgx: Fix race between reclaimer and page fault handler Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 61/75] x86/sgx: Ensure no data in PCMD page after truncate Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 62/75] media: i2c: imx412: Fix reset GPIO polarity Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 63/75] media: i2c: imx412: Fix power_off ordering Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 64/75] tpm: Fix buffer access in tpm2_get_tpm_pt() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 65/75] tpm: ibmvtpm: Correct the return value in tpm_ibmvtpm_probe() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 66/75] docs: submitting-patches: Fix crossref to The canonical patch format Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 67/75] NFS: Memory allocation failures are not server fatal errors Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 68/75] NFSD: Fix possible sleep during nfsd4_release_lockowner() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 69/75] bpf: Fix potential array overflow in bpf_trampoline_get_progs() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 70/75] bpf: Fix combination of jit blinding and pointers to bpf subprogs Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 71/75] bpf: Enlarge offset check value to INT_MAX in bpf_skb_{load,store}_bytes Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 72/75] bpf: Fix usage of trace RCU in local storage Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 73/75] bpf: Fix excessive memory allocation in stack_map_alloc() Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 74/75] bpf: Reject writes for PTR_TO_MAP_KEY in check_helper_mem_access Greg Kroah-Hartman
2022-06-03 17:43 ` [PATCH 5.17 75/75] bpf: Check PTR_TO_MEM | MEM_RDONLY " Greg Kroah-Hartman
2022-06-03 22:20 ` [PATCH 5.17 00/75] 5.17.13-rc1 review Fox Chen
2022-06-03 22:57 ` Justin Forbes
2022-06-04 12:48 ` Sudip Mukherjee
2022-06-04 16:40 ` Naresh Kamboju
2022-06-04 18:55 ` Guenter Roeck
2022-06-04 19:32 ` Ron Economos

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220603173823.410844930@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=dave.hansen@linux.intel.com \
    --cc=haitao.huang@intel.com \
    --cc=jarkko@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=reinette.chatre@intel.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox