From: "Haitao Huang" <haitao.huang@linux.intel.com>
To: linux-sgx@vger.kernel.org, "Jarkko Sakkinen" <jarkko@kernel.org>
Cc: dave.hansen@intel.com,
"Dave Hansen" <dave.hansen@linux.intel.com>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Ingo Molnar" <mingo@redhat.com>,
"Borislav Petkov" <bp@alien8.de>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
"Sean Christopherson" <seanjc@google.com>,
"Jethro Beekman" <jethro@fortanix.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v8] x86/sgx: Maintain encl->refcount for each encl->mm_list entry
Date: Tue, 13 Apr 2021 20:15:09 -0500 [thread overview]
Message-ID: <op.01te3jzwwjvjmi@mqcpg7oapc828.gar.corp.intel.com> (raw)
In-Reply-To: <20210207221401.29933-1-jarkko@kernel.org>
On Sun, 07 Feb 2021 16:14:01 -0600, Jarkko Sakkinen <jarkko@kernel.org>
wrote:
> 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.
>
The comments in code around this warning indicate a potential memory leak.
Not sure how use-after-free come into play. Anyway, this fix seems to work
for the warning above.
However, I still have doubts on another potential race. See below.
> 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)
> synchronize_srcu(&encl->srcu);
> mmu_notifier_unregister(&encl_mm->mmu_notifier, encl_mm->mm);
> kfree(encl_mm);
Note here you are freeing the encl_mm, outside protection of
encl->refcount.
> +
> + /* '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);
> +
> kfree(encl_mm);
Could this access to and kfree of encl_mm possibly be after the
kfree(encl_mm) noted above?
Also is there a reason we do kfree(encl_mm) in notifier_free not directly
in notifier_release?
Thanks
Haitao
next prev parent reply other threads:[~2021-04-14 1:15 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-07 22:14 [PATCH v8] x86/sgx: Maintain encl->refcount for each encl->mm_list entry Jarkko Sakkinen
2021-02-07 22:37 ` Dave Hansen
2021-02-08 18:20 ` [tip: x86/urgent] " tip-bot2 for Jarkko Sakkinen
2021-04-14 1:15 ` Haitao Huang [this message]
2021-04-14 15:51 ` [PATCH v8] " 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:
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=op.01te3jzwwjvjmi@mqcpg7oapc828.gar.corp.intel.com \
--to=haitao.huang@linux.intel.com \
--cc=bp@alien8.de \
--cc=dave.hansen@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=jarkko@kernel.org \
--cc=jethro@fortanix.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sgx@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=seanjc@google.com \
--cc=tglx@linutronix.de \
--cc=x86@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 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.