linux-kernel - [tip: x86/sgx] x86/sgx: Avoid using iterator after loop in sgx_mmu_notifier

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <168666719541.404.14580547563993446392.tip-bot2@tip-bot2>
Date:   Tue, 13 Jun 2023 14:39:55 -0000
From:   "tip-bot2 for Jakob Koschel" <tip-bot2@...utronix.de>
To:     linux-tip-commits@...r.kernel.org
Cc:     Jakob Koschel <jkl820.git@...il.com>,
        "Borislav Petkov (AMD)" <bp@...en8.de>,
        Jarkko Sakkinen <jarkko@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: [tip: x86/sgx] x86/sgx: Avoid using iterator after loop in
 sgx_mmu_notifier_release()

The following commit has been merged into the x86/sgx branch of tip:

Commit-ID:     1e327963cfab0e02eeeb0331178d6c353c959cd6
Gitweb:        https://git.kernel.org/tip/1e327963cfab0e02eeeb0331178d6c353c959cd6
Author:        Jakob Koschel <jkl820.git@...il.com>
AuthorDate:    Wed, 01 Mar 2023 12:17:29 +01:00
Committer:     Borislav Petkov (AMD) <bp@...en8.de>
CommitterDate: Tue, 13 Jun 2023 16:21:01 +02:00

x86/sgx: Avoid using iterator after loop in sgx_mmu_notifier_release()

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 macro to avoid any potential misuse after the loop. Using it in
a pointer comparison after the loop is undefined behavior and should be
omitted if possible, see Link tag.

Instead, just use a 'found' boolean to indicate if an element was found.

  [ bp: Massage, fix typos. ]

Signed-off-by: Jakob Koschel <jkl820.git@...il.com>
Signed-off-by: Borislav Petkov (AMD) <bp@...en8.de>
Reviewed-by: Jarkko Sakkinen <jarkko@...nel.org>
Acked-by: Dave Hansen <dave.hansen@...ux.intel.com>
Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/
Link: https://lore.kernel.org/r/20230206-sgx-use-after-iter-v2-1-736ca621adc3@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 2a0e90f..91fa70e 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);
 	}