linux-kernel - [PATCH] x86/sgx: Suppress WARN on inability to sanitize EPC if ksgxd is stopped

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: <20210616004458.2192889-1-seanjc@google.com>
Date:   Tue, 15 Jun 2021 17:44:58 -0700
From:   Sean Christopherson <seanjc@...gle.com>
To:     Jarkko Sakkinen <jarkko@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org
Cc:     Dave Hansen <dave.hansen@...ux.intel.com>,
        "H. Peter Anvin" <hpa@...or.com>, linux-sgx@...r.kernel.org,
        linux-kernel@...r.kernel.org, Du Cheng <ducheng2@...il.com>,
        Sean Christopherson <seanjc@...gle.com>
Subject: [PATCH] x86/sgx: Suppress WARN on inability to sanitize EPC if ksgxd
 is stopped

Don't WARN on having unsanitized EPC pages if ksgxd is stopped early,
e.g. if sgx_init() realizes there will be no downstream consumers of EPC.
If ksgxd is stopped early, EPC pages may be left on the dirty list, but
that's ok because ksgxd is only stopped if SGX initialization failed or
if the kernel is going down.  In either case, the EPC won't be used.

This bug was exposed by the addition of KVM support, but has existed and
was hittable since the original sanitization code was added.  Prior to
adding KVM support, if Launch Control was not fully enabled, e.g. when
running on older hardware, sgx_init() bailed immediately before spawning
ksgxd because X86_FEATURE_SGX was cleared if X86_FEATURE_SGX_LC was
unsupported.

With KVM support, sgx_drv_init() handles the X86_FEATURE_SGX_LC check
manually, so now there's any easy-to-hit case where sgx_init() will spawn
ksgxd and _then_ fail to initialize, which results in sgx_init() stopping
ksgxd before it finishes sanitizing the EPC.

Prior to KVM support, the bug was much harder to hit because it basically
required char device registration to fail.

Reported-by: Du Cheng <ducheng2@...il.com>
Fixes: e7e0545299d8 ("x86/sgx: Initialize metadata for Enclave Page Cache (EPC) sections")
Signed-off-by: Sean Christopherson <seanjc@...gle.com>
---

Lightly tested due to lack of hardware.  I hacked the flow to verify that
stopping early will leave work pending, and that rechecking should_stop()
suppress the resulting WARN.

 arch/x86/kernel/cpu/sgx/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index ad904747419e..fbad2b9625a5 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -425,7 +425,7 @@ static int ksgxd(void *p)
 	__sgx_sanitize_pages(&sgx_dirty_page_list);

 	/* sanity check: */
-	WARN_ON(!list_empty(&sgx_dirty_page_list));
+	WARN_ON(!list_empty(&sgx_dirty_page_list) && !kthread_should_stop());

 	while (!kthread_should_stop()) {
 		if (try_to_freeze())
-- 
2.32.0.272.g935e593368-goog