Hi Jarkko,
On Thu, 25 Aug 2022 03:08:02 -0500, Jarkko Sakkinen <jarkko@xxxxxxxxxx>
wrote:
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@xxxxxxxxxx/T/#u
Reported-by: Paul Menzel <pmenzel@xxxxxxxxxxxxx>
Fixes: 51ab30eb2ad4 ("x86/sgx: Replace section->init_laundry_list with
sgx_dirty_page_list")
Signed-off-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
Cc: Haitao Huang <haitao.huang@xxxxxxxxxxxxxxx>
Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
Cc: Reinette Chatre <reinette.chatre@xxxxxxxxx>
---
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: */
Just a nitpick,
Although it is not added in this patch, the above comment is not accurate.
The list is accessed one thread only: filled first in main thread, then
only ever accessed here.
IIUC, could you remove or update that comment?
Other than that, FWIW:
Reviewed-by: Haitao Huang <haitao.huang@xxxxxxxxxxxxxxx>
Thanks
Haitao
