From: Jakob Koschel <jkl820.git@gmail.com>
To: Jarkko Sakkinen <jarkko@kernel.org>,
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>
Cc: linux-sgx@vger.kernel.org, linux-kernel@vger.kernel.org,
Pietro Borrello <borrello@diag.uniroma1.it>,
Cristiano Giuffrida <c.giuffrida@vu.nl>,
"Bos, H.J." <h.j.bos@vu.nl>, Jakob Koschel <jkl820.git@gmail.com>
Subject: [PATCH v2] x86/sgx: Avoid using iterator after loop in sgx_mmu_notifier_release()
Date: Wed, 01 Mar 2023 12:17:29 +0100 [thread overview]
Message-ID: <20230206-sgx-use-after-iter-v2-1-736ca621adc3@gmail.com> (raw)
If &encl_mm->encl->mm_list does not contain the searched 'encl_mm',
'tmp' will not point to a valid sgx_encl_mm struct.
Linus proposed to avoid any use of the list iterator variable after the
loop, in the attempt to move the list iterator variable declaration into
the marcro to avoid any potential misuse after the loop.
Using it in a pointer comparision after the loop is undefined behavior
and should be omitted if possible [1].
Instead we'll just use a 'found' boolean to indicate if an element
was found.
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <jkl820.git@gmail.com>
---
Changes in v2:
- refactor to use 'found' variable instead of moving code into the loop
- add cover letter info into the patch
- Link to v1: https://lore.kernel.org/r/20230206-sgx-use-after-iter-v1-1-c09fb5300b5e@gmail.com
---
arch/x86/kernel/cpu/sgx/encl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
index 2a0e90fe2abc..91fa70e51004 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -755,6 +755,7 @@ static void sgx_mmu_notifier_release(struct mmu_notifier *mn,
{
struct sgx_encl_mm *encl_mm = container_of(mn, struct sgx_encl_mm, mmu_notifier);
struct sgx_encl_mm *tmp = NULL;
+ bool found = false;
/*
* The enclave itself can remove encl_mm. Note, objects can't be moved
@@ -764,12 +765,13 @@ static void sgx_mmu_notifier_release(struct mmu_notifier *mn,
list_for_each_entry(tmp, &encl_mm->encl->mm_list, list) {
if (tmp == encl_mm) {
list_del_rcu(&encl_mm->list);
+ found = true;
break;
}
}
spin_unlock(&encl_mm->encl->mm_lock);
- if (tmp == encl_mm) {
+ if (found) {
synchronize_srcu(&encl_mm->encl->srcu);
mmu_notifier_put(mn);
}
---
base-commit: d2d11f342b179f1894a901f143ec7c008caba43e
change-id: 20230206-sgx-use-after-iter-f584c1d64c87
Best regards,
--
Jakob Koschel <jkl820.git@gmail.com>
next reply other threads:[~2023-03-01 11:18 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-01 11:17 Jakob Koschel [this message]
2023-03-01 23:07 ` [PATCH v2] x86/sgx: Avoid using iterator after loop in sgx_mmu_notifier_release() Jarkko Sakkinen
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=20230206-sgx-use-after-iter-v2-1-736ca621adc3@gmail.com \
--to=jkl820.git@gmail.com \
--cc=borrello@diag.uniroma1.it \
--cc=bp@alien8.de \
--cc=c.giuffrida@vu.nl \
--cc=dave.hansen@linux.intel.com \
--cc=h.j.bos@vu.nl \
--cc=hpa@zytor.com \
--cc=jarkko@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sgx@vger.kernel.org \
--cc=mingo@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).