linux-kernel - [PATCH] x86/shstk: Don't retry vm

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230706233858.446232-1-rick.p.edgecombe@intel.com>
Date:   Thu,  6 Jul 2023 16:38:58 -0700
From:   Rick Edgecombe <rick.p.edgecombe@...el.com>
To:     rick.p.edgecombe@...el.com
Cc:     akpm@...ux-foundation.org, andrew.cooper3@...rix.com,
        arnd@...db.de, bp@...en8.de, broonie@...nel.org,
        bsingharora@...il.com, christina.schimpe@...el.com, corbet@....net,
        dave.hansen@...el.com, dave.hansen@...ux.intel.com,
        david@...hat.com, debug@...osinc.com, dethoma@...rosoft.com,
        eranian@...gle.com, esyr@...hat.com, fweimer@...hat.com,
        gorcunov@...il.com, hjl.tools@...il.com, hpa@...or.com,
        jamorris@...ux.microsoft.com, jannh@...gle.com, john.allen@....com,
        kcc@...gle.com, keescook@...omium.org,
        kirill.shutemov@...ux.intel.com, linux-api@...r.kernel.org,
        linux-arch@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org, luto@...nel.org,
        mike.kravetz@...cle.com, mingo@...hat.com, nadav.amit@...il.com,
        oleg@...hat.com, pavel@....cz, pengfei.xu@...el.com,
        peterz@...radead.org, rdunlap@...radead.org, rppt@...nel.org,
        szabolcs.nagy@....com, tglx@...utronix.de,
        torvalds@...ux-foundation.org, weijiang.yang@...el.com,
        x86@...nel.org, yu-cheng.yu@...el.com
Subject: [PATCH] x86/shstk: Don't retry vm_munmap() on -EINTR

The existing comment around handling vm_munmap() failure when freeing a
shadow stack is wrong. It asserts that vm_munmap() returns -EINTR when
the mmap lock is only being held for a short time, and so the caller
should retry. Based on this wrong understanding, unmap_shadow_stack() will
loop retrying vm_munmap().

What -EINTR actually means in this case is that the process is going
away (see ae79878), and the whole MM will be torn down soon. In order
to facilitate this, the task should not linger in the kernel, but
actually do the opposite. So don't loop in this scenario, just abandon
the operation and let exit_mmap() clean it up. Also, update the comment
to reflect the actual meaning of the error code.

Signed-off-by: Rick Edgecombe <rick.p.edgecombe@...el.com>
---
 arch/x86/kernel/shstk.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kernel/shstk.c b/arch/x86/kernel/shstk.c
index 47f5204b0fa9..cd10d074a444 100644
--- a/arch/x86/kernel/shstk.c
+++ b/arch/x86/kernel/shstk.c
@@ -134,28 +134,24 @@ static unsigned long adjust_shstk_size(unsigned long size)
 
 static void unmap_shadow_stack(u64 base, u64 size)
 {
-	while (1) {
-		int r;
+	int r;
 
-		r = vm_munmap(base, size);
+	r = vm_munmap(base, size);
 
-		/*
-		 * vm_munmap() returns -EINTR when mmap_lock is held by
-		 * something else, and that lock should not be held for a
-		 * long time.  Retry it for the case.
-		 */
-		if (r == -EINTR) {
-			cond_resched();
-			continue;
-		}
+	/*
+	 * mmap_write_lock_killable() failed with -EINTR. This means
+	 * the process is about to die and have it's MM cleaned up.
+	 * This task shouldn't ever make it back to userspace. In this
+	 * case it is ok to leak a shadow stack, so just exit out.
+	 */
+	if (r == -EINTR)
+		return;
 
-		/*
-		 * For all other types of vm_munmap() failure, either the
-		 * system is out of memory or there is bug.
-		 */
-		WARN_ON_ONCE(r);
-		break;
-	}
+	/*
+	 * For all other types of vm_munmap() failure, either the
+	 * system is out of memory or there is bug.
+	 */
+	WARN_ON_ONCE(r);
 }
 
 static int shstk_setup(void)
-- 
2.34.1