linux-kernel - [PATCH] 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-next>] [day] [month] [year] [list]

Message-Id: <20230206-sgx-use-after-iter-v1-1-c09fb5300b5e@gmail.com>
Date:   Mon, 06 Feb 2023 11:39:02 +0100
From:   Jakob Koschel <jkl820.git@...il.com>
To:     Jarkko Sakkinen <jarkko@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>
Cc:     linux-sgx@...r.kernel.org, linux-kernel@...r.kernel.org,
        Pietro Borrello <borrello@...g.uniroma1.it>,
        Cristiano Giuffrida <c.giuffrida@...nl>,
        "Bos, H.J." <h.j.bos@...nl>, Jakob Koschel <jkl820.git@...il.com>
Subject: [PATCH] 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.

Since the code within the guarded block is just called when the element
is found, it can simply be moved into the list iterator.
Within the list iterator 'tmp' is guaranteed to point to a valid
element.

Signed-off-by: Jakob Koschel <jkl820.git@...il.com>
---
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].

Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
---
 arch/x86/kernel/cpu/sgx/encl.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
index 2a0e90fe2abc..db585b780141 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -764,15 +764,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);
-			break;
+			spin_unlock(&encl_mm->encl->mm_lock);
+			synchronize_srcu(&encl_mm->encl->srcu);
+			mmu_notifier_put(mn);
+			return;
 		}
 	}
 	spin_unlock(&encl_mm->encl->mm_lock);
-
-	if (tmp == encl_mm) {
-		synchronize_srcu(&encl_mm->encl->srcu);
-		mmu_notifier_put(mn);
-	}
 }
 
 static void sgx_mmu_notifier_free(struct mmu_notifier *mn)

---
base-commit: d2d11f342b179f1894a901f143ec7c008caba43e
change-id: 20230206-sgx-use-after-iter-f584c1d64c87

Best regards,
-- 
Jakob Koschel <jkl820.git@...il.com>