All of
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <>
Cc:, Jarkko Sakkinen <>,
	Haitao Huang <>,
	Dave Hansen <>,
	Thomas Gleixner <>,
	Ingo Molnar <>, Borislav Petkov <>,, "H. Peter Anvin" <>,
	Sean Christopherson <>,
	Jethro Beekman <>,
Subject: [PATCH v8] x86/sgx: Maintain encl->refcount for each encl->mm_list entry
Date: Mon,  8 Feb 2021 00:14:01 +0200	[thread overview]
Message-ID: <> (raw)

This has been shown in tests:

[  +0.000008] WARNING: CPU: 3 PID: 7620 at kernel/rcu/srcutree.c:374 cleanup_srcu_struct+0xed/0x100

This is essentially a use-after free, although SRCU notices it as
an SRCU cleanup in an invalid context.

== Background ==

SGX has a data structure (struct sgx_encl_mm) which keeps per-mm SGX
metadata.  This is separate from 'struct sgx_encl' because, in theory,
an enclave can be mapped from more than one mm.  sgx_encl_mm includes
a pointer back to the sgx_encl.

This means that sgx_encl must have a longer lifetime than all of the
sgx_encl_mm's that point to it.  That's usually the case: sgx_encl_mm
is freed only after the mmu_notifier is unregistered in sgx_release().

However, there's a race.  If the process is exiting,
sgx_mmu_notifier_release() can be called in parallel with sgx_release()
instead of being called *by* it.  The mmu_notifier path keeps encl_mm
alive past when sgx_encl can be freed.  This inverts the lifetime rules
and means that sgx_mmu_notifier_release() can access a freed sgx_encl.

== Fix ==

Increase encl->refcount when encl_mm->encl is established. Release
this reference encl_mm is freed.  This ensures that 'encl' outlives

Fixes: 1728ab54b4be ("x86/sgx: Add a page reclaimer")
Cc: Dave Hansen <
Reported-by: Haitao Huang <>
Signed-off-by: Jarkko Sakkinen <>
- Slight adjustments on call sites suggested by Dave, to make things
  more clear and obvious. Otherwise, semantically same as v7:
  Contains also long description written by Dave.
- No changes from v6. Resend of
- Maintain refcount for each encl->mm_list entry.
- To make sure that the instance does not get deleted use kref_get()
  kref_put(). This also removes the need for additional
- Rewrite the commit message.
- Just change the call order. *_expedited() is out of scope for this
  bug fix.
v3: Fine-tuned tags, and added missing change log for v2.
v2: Switch to synchronize_srcu_expedited().

 arch/x86/kernel/cpu/sgx/driver.c | 3 +++
 arch/x86/kernel/cpu/sgx/encl.c   | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/arch/x86/kernel/cpu/sgx/driver.c b/arch/x86/kernel/cpu/sgx/driver.c
index f2eac41bb4ff..8ce6d8371cfb 100644
--- a/arch/x86/kernel/cpu/sgx/driver.c
+++ b/arch/x86/kernel/cpu/sgx/driver.c
@@ -72,6 +72,9 @@ static int sgx_release(struct inode *inode, struct file *file)
 		mmu_notifier_unregister(&encl_mm->mmu_notifier, encl_mm->mm);
+		/* 'encl_mm' is gone, put encl_mm->encl reference: */
+		kref_put(&encl->refcount, sgx_encl_release);
 	kref_put(&encl->refcount, sgx_encl_release);
diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
index 20a2dd5ba2b4..7449ef33f081 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -473,6 +473,9 @@ static void sgx_mmu_notifier_free(struct mmu_notifier *mn)
 	struct sgx_encl_mm *encl_mm = container_of(mn, struct sgx_encl_mm, mmu_notifier);
+	/* 'encl_mm' is going away, put encl_mm->encl reference: */
+	kref_put(&encl_mm->encl->refcount, sgx_encl_release);
@@ -526,6 +529,8 @@ int sgx_encl_mm_add(struct sgx_encl *encl, struct mm_struct *mm)
 	if (!encl_mm)
 		return -ENOMEM;
+	/* Grab a refcount for the encl_mm->encl reference: */
+	kref_get(&encl->refcount);
 	encl_mm->encl = encl;
 	encl_mm->mm = mm;
 	encl_mm->mmu_notifier.ops = &sgx_mmu_notifier_ops;

             reply	other threads:[~2021-02-07 22:15 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-07 22:14 Jarkko Sakkinen [this message]
2021-02-07 22:37 ` [PATCH v8] x86/sgx: Maintain encl->refcount for each encl->mm_list entry Dave Hansen
2021-02-08 18:20 ` [tip: x86/urgent] " tip-bot2 for Jarkko Sakkinen
2021-04-14  1:15 ` [PATCH v8] " Haitao Huang
2021-04-14 15:51   ` Sean Christopherson
2021-04-14 16:58     ` Dave Hansen

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:

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

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.