[PATCH v3] x86/sgx: Do not consider unsanitized pages an error

Jarkko Sakkinen
2022-08-25T08:08:35Z

If sgx_dirty_page_list ends up being non-empty, currently this triggers
WARN_ON(), which produces a lot of noise, and can potentially crash the
kernel, depending on the kernel command line.

However, if the SGX subsystem initialization is retracted, the sanitization
process could end up in the middle, and sgx_dirty_page_list be left
non-empty for legit reasons.

Replace this faulty behavior with more verbose version
__sgx_sanitize_pages(), which can optionally print EREMOVE error code and
the number of unsanitized pages.

Link: https://lore.kernel.org/linux-sgx/20220825051827.246698-1-jarkko@kernel.org/T/#u
Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
Fixes: 51ab30eb2ad4 ("x86/sgx: Replace section->init_laundry_list with sgx_dirty_page_list")
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

Cc: Haitao Huang <haitao.huang@linux.intel.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Reinette Chatre <reinette.chatre@intel.com>
---
v3:
- Remove WARN_ON().
- Tuned comments and the commit message a bit.

v2:
- Replaced WARN_ON() with optional pr_info() inside
__sgx_sanitize_pages().
- Rewrote the commit message.
- Added the fixes tag.
---
arch/x86/kernel/cpu/sgx/main.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
index 515e2a5f25bb..d204520a5e26 100644
--- a/arch/x86/kernel/cpu/sgx/main.c
+++ b/arch/x86/kernel/cpu/sgx/main.c
@@ -50,16 +50,17 @@ static LIST_HEAD(sgx_dirty_page_list);
* from the input list, and made available for the page allocator. SECS pages
* prepending their children in the input list are left intact.
*/
-static void __sgx_sanitize_pages(struct list_head *dirty_page_list)
+static void __sgx_sanitize_pages(struct list_head *dirty_page_list, bool verbose)
{
struct sgx_epc_page *page;
+ int dirty_count = 0;
LIST_HEAD(dirty);
int ret;

/* dirty_page_list is thread-local, no need for a lock: */
while (!list_empty(dirty_page_list)) {
if (kthread_should_stop())
- return;
+ break;

page = list_first_entry(dirty_page_list, struct sgx_epc_page, list);

@@ -90,14 +91,22 @@ static void __sgx_sanitize_pages(struct list_head *dirty_page_list)
list_del(&page->list);
sgx_free_epc_page(page);
} else {
+ if (verbose)
+ pr_err_ratelimited(EREMOVE_ERROR_MESSAGE, ret, ret);
+
/* The page is not yet clean - move to the dirty list. */
list_move_tail(&page->list, &dirty);
+ dirty_count++;
}

cond_resched();
}

list_splice(&dirty, dirty_page_list);
+
+ /* Can happen, when the initialization is retracted: */
+ if (verbose && dirty_count > 0)
+ pr_info("%d unsanitized pages\n", dirty_count);
}

static bool sgx_reclaimer_age(struct sgx_epc_page *epc_page)
@@ -394,11 +403,8 @@ static int ksgxd(void *p)
* Sanitize pages in order to recover from kexec(). The 2nd pass is
* required for SECS pages, whose child pages blocked EREMOVE.
*/
- __sgx_sanitize_pages(&sgx_dirty_page_list);
- __sgx_sanitize_pages(&sgx_dirty_page_list);
-
- /* sanity check: */
- WARN_ON(!list_empty(&sgx_dirty_page_list));
+ __sgx_sanitize_pages(&sgx_dirty_page_list, false);
+ __sgx_sanitize_pages(&sgx_dirty_page_list, true);

while (!kthread_should_stop()) {
if (try_to_freeze())
--
2.37.1