On Sun, 07 Feb 2021 16:14:01 -0600, Jarkko Sakkinen <jarkko@xxxxxxxxxx>
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
