| Message ID | 20220906000221.34286-2-jarkko@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Critical fixes for v6.0 | expand |
On Tue, 2022-09-06 at 03:02 +0300, Jarkko Sakkinen wrote: > Unsanitized pages trigger WARN_ON() unconditionally, which can panic the > whole computer, if /proc/sys/kernel/panic_on_warn is set. > > In sgx_init(), if misc_register() fails or misc_register() succeeds but > neither sgx_drv_init() nor sgx_vepc_init() succeeds, then ksgxd will be > prematurely stopped. This may leave unsanitized pages, which will result a > false warning. > > Refine __sgx_sanitize_pages() to return: > > 1. Zero when the sanitization process is complete or ksgxd has been > requested to stop. > 2. The number of unsanitized pages otherwise. > > Link: https://lore.kernel.org/linux-sgx/20220825051827.246698-1-jarkko@kernel.org/T/#u > Fixes: 51ab30eb2ad4 ("x86/sgx: Replace section->init_laundry_list with sgx_dirty_page_list") > Cc: stable@vger.kernel.org # v5.13+ > Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> (Given the idea of moving sgx_page_reclaimer_init() to the end of sgx_init() is considered too big to fix this bug:) Acked-by: Kai Huang <kai.huang@intel.com>
On 9/5/2022 5:02 PM, Jarkko Sakkinen wrote: > Unsanitized pages trigger WARN_ON() unconditionally, which can panic the > whole computer, if /proc/sys/kernel/panic_on_warn is set. > > In sgx_init(), if misc_register() fails or misc_register() succeeds but > neither sgx_drv_init() nor sgx_vepc_init() succeeds, then ksgxd will be > prematurely stopped. This may leave unsanitized pages, which will result a > false warning. > > Refine __sgx_sanitize_pages() to return: > > 1. Zero when the sanitization process is complete or ksgxd has been > requested to stop. > 2. The number of unsanitized pages otherwise. > > Link: https://lore.kernel.org/linux-sgx/20220825051827.246698-1-jarkko@kernel.org/T/#u > Fixes: 51ab30eb2ad4 ("x86/sgx: Replace section->init_laundry_list with sgx_dirty_page_list") > Cc: stable@vger.kernel.org # v5.13+ > Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> Reviewed-by: Reinette Chatre <reinette.chatre@intel.com> Reinette
On Thu, Sep 08, 2022 at 12:53:55PM -0700, Reinette Chatre wrote: > > > On 9/5/2022 5:02 PM, Jarkko Sakkinen wrote: > > Unsanitized pages trigger WARN_ON() unconditionally, which can panic the > > whole computer, if /proc/sys/kernel/panic_on_warn is set. > > > > In sgx_init(), if misc_register() fails or misc_register() succeeds but > > neither sgx_drv_init() nor sgx_vepc_init() succeeds, then ksgxd will be > > prematurely stopped. This may leave unsanitized pages, which will result a > > false warning. > > > > Refine __sgx_sanitize_pages() to return: > > > > 1. Zero when the sanitization process is complete or ksgxd has been > > requested to stop. > > 2. The number of unsanitized pages otherwise. > > > > Link: https://lore.kernel.org/linux-sgx/20220825051827.246698-1-jarkko@kernel.org/T/#u > > Fixes: 51ab30eb2ad4 ("x86/sgx: Replace section->init_laundry_list with sgx_dirty_page_list") > > Cc: stable@vger.kernel.org # v5.13+ > > Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> > > Reviewed-by: Reinette Chatre <reinette.chatre@intel.com> Thanks. I also split down the long augment test also into pieces now, as you requested, and I think it is now somewhat clean. BR, Jarkko
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index 515e2a5f25bb..0aad028f04d4 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -49,9 +49,13 @@ static LIST_HEAD(sgx_dirty_page_list); * Reset post-kexec EPC pages to the uninitialized state. The pages are removed * from the input list, and made available for the page allocator. SECS pages * prepending their children in the input list are left intact. + * + * Return 0 when sanitization was successful or kthread was stopped, and the + * number of unsanitized pages otherwise. */ -static void __sgx_sanitize_pages(struct list_head *dirty_page_list) +static unsigned long __sgx_sanitize_pages(struct list_head *dirty_page_list) { + unsigned long left_dirty = 0; struct sgx_epc_page *page; LIST_HEAD(dirty); int ret; @@ -59,7 +63,7 @@ static void __sgx_sanitize_pages(struct list_head *dirty_page_list) /* dirty_page_list is thread-local, no need for a lock: */ while (!list_empty(dirty_page_list)) { if (kthread_should_stop()) - return; + return 0; page = list_first_entry(dirty_page_list, struct sgx_epc_page, list); @@ -92,12 +96,14 @@ static void __sgx_sanitize_pages(struct list_head *dirty_page_list) } else { /* The page is not yet clean - move to the dirty list. */ list_move_tail(&page->list, &dirty); + left_dirty++; } cond_resched(); } list_splice(&dirty, dirty_page_list); + return left_dirty; } static bool sgx_reclaimer_age(struct sgx_epc_page *epc_page) @@ -395,10 +401,7 @@ static int ksgxd(void *p) * 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)); + WARN_ON(__sgx_sanitize_pages(&sgx_dirty_page_list)); while (!kthread_should_stop()) { if (try_to_freeze())
Unsanitized pages trigger WARN_ON() unconditionally, which can panic the whole computer, if /proc/sys/kernel/panic_on_warn is set. In sgx_init(), if misc_register() fails or misc_register() succeeds but neither sgx_drv_init() nor sgx_vepc_init() succeeds, then ksgxd will be prematurely stopped. This may leave unsanitized pages, which will result a false warning. Refine __sgx_sanitize_pages() to return: 1. Zero when the sanitization process is complete or ksgxd has been requested to stop. 2. The number of unsanitized pages otherwise. Link: https://lore.kernel.org/linux-sgx/20220825051827.246698-1-jarkko@kernel.org/T/#u Fixes: 51ab30eb2ad4 ("x86/sgx: Replace section->init_laundry_list with sgx_dirty_page_list") Cc: stable@vger.kernel.org # v5.13+ Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> --- v9: - Remove left_dirty declaratio from ksgxd(). - Update commit message. v8: - Discard changes that are not relevant for the stable fix. This does absolutely minimum to address the bug: https://lore.kernel.org/linux-sgx/a5fa56bdc57d6472a306bd8d795afc674b724538.camel@intel.com/ v7: - Rewrote commit message. - Do not return -ECANCELED on premature stop. Instead use zero both premature stop and complete sanitization. v6: - Address Reinette's feedback: https://lore.kernel.org/linux-sgx/Yw6%2FiTzSdSw%2FY%2FVO@kernel.org/ v5: - Add the klog dump and sysctl option to the commit message. v4: - Explain expectations for dirty_page_list in the function header, instead of an inline comment. - Improve commit message to explain the conditions better. - Return the number of pages left dirty to ksgxd() and print warning after the 2nd call, if there are any. 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 | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)