[RFC PATCH v2 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup

From: Bojun Zhu
Date: Fri Apr 26 2024 - 10:25:24 EST


EDMM's ioctl()s support batch operations, which may be
time-consuming. Try to explicitly give up the CPU as the prefix
operation at the every begin of "for loop" in
sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
to give other tasks a chance to run, and avoid softlockup warning.

Additionally perform pending signals check as the prefix operation,
and introduce sgx_check_signal_and_resched(),
which wraps all the checks.

The following has been observed on Linux v6.9-rc5 with kernel
preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
is requested to restrict page permissions of a large number of EPC pages.

------------[ cut here ]------------
watchdog: BUG: soft lockup - CPU#45 stuck for 22s!
...
RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
...
Call Trace:
sgx_ioctl
__x64_sys_ioctl
x64_sys_call
do_syscall_64
entry_SYSCALL_64_after_hwframe
------------[ end trace ]------------

Signed-off-by: Bojun Zhu <zhubojun.zbj@xxxxxxxxxxxx>
---
arch/x86/kernel/cpu/sgx/ioctl.c | 40 +++++++++++++++++++++++++++++----
1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index b65ab214bdf5..e0645920bcb5 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -365,6 +365,20 @@ static int sgx_validate_offset_length(struct sgx_encl *encl,
return 0;
}

+/*
+ * Check signals and invoke scheduler. Return true for a pending signal.
+ */
+static bool sgx_check_signal_and_resched(void)
+{
+ if (signal_pending(current))
+ return true;
+
+ if (need_resched())
+ cond_resched();
+
+ return false;
+}
+
/**
* sgx_ioc_enclave_add_pages() - The handler for %SGX_IOC_ENCLAVE_ADD_PAGES
* @encl: an enclave pointer
@@ -432,16 +446,13 @@ static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg)
return -EINVAL;

for (c = 0 ; c < add_arg.length; c += PAGE_SIZE) {
- if (signal_pending(current)) {
+ if (sgx_check_signal_and_resched()) {
if (!c)
ret = -ERESTARTSYS;

break;
}

- if (need_resched())
- cond_resched();
-
ret = sgx_encl_add_page(encl, add_arg.src + c, add_arg.offset + c,
&secinfo, add_arg.flags);
if (ret)
@@ -746,6 +757,13 @@ sgx_enclave_restrict_permissions(struct sgx_encl *encl,
secinfo.flags = modp->permissions & SGX_SECINFO_PERMISSION_MASK;

for (c = 0 ; c < modp->length; c += PAGE_SIZE) {
+ if (sgx_check_signal_and_resched()) {
+ if (!c)
+ ret = -ERESTARTSYS;
+
+ goto out;
+ }
+
addr = encl->base + modp->offset + c;

sgx_reclaim_direct();
@@ -913,6 +931,13 @@ static long sgx_enclave_modify_types(struct sgx_encl *encl,
secinfo.flags = page_type << 8;

for (c = 0 ; c < modt->length; c += PAGE_SIZE) {
+ if (sgx_check_signal_and_resched()) {
+ if (!c)
+ ret = -ERESTARTSYS;
+
+ goto out;
+ }
+
addr = encl->base + modt->offset + c;

sgx_reclaim_direct();
@@ -1101,6 +1126,13 @@ static long sgx_encl_remove_pages(struct sgx_encl *encl,
secinfo.flags = SGX_SECINFO_R | SGX_SECINFO_W | SGX_SECINFO_X;

for (c = 0 ; c < params->length; c += PAGE_SIZE) {
+ if (sgx_check_signal_and_resched()) {
+ if (!c)
+ ret = -ERESTARTSYS;
+
+ goto out;
+ }
+
addr = encl->base + params->offset + c;

sgx_reclaim_direct();
--
2.25.1