__restore_processor_context() had a couple of ordering bugs.  It
restored GSBASE after calling load_gs_index(), and the latter can
call into tracing code.  It also tried to restore segment registers
before restoring the LDT, which is straight-up wrong.

Reorder the code so that we restore GSBASE, then the descriptor
tables, then the segments.

This fixes two bugs.  First, it fixes a regression that broke resume
under certain configurations due to irqflag tracing in
native_load_gs_index().  Second, it fixes resume when the userspace
process that initiated suspect had funny segments.  The latter can be
reproduced by compiling this:

// SPDX-License-Identifier: GPL-2.0
/*
 * ldt_echo.c - Echo argv[1] while using an LDT segment
 */

int main(int argc, char **argv)
{
        int ret;
        size_t len;
        char *buf;

        const struct user_desc desc = {
                .entry_number    = 0,
                .base_addr       = 0,
                .limit           = 0xfffff,
                .seg_32bit       = 1,
                .contents        = 0, /* Data, grow-up */
                .read_exec_only  = 0,
                .limit_in_pages  = 1,
                .seg_not_present = 1,
                .useable         = 0
        };

        if (argc != 2)
                errx(1, "Usage: %s STRING", argv[0]);

        len = asprintf(&buf, "%s\n", argv[1]);
        if (len < 0)
                errx(1, "Out of memory");

        ret = syscall(SYS_modify_ldt, 1, &desc, sizeof(desc));
        if (ret < -1)
                errno = -ret;
        if (ret)
                err(1, "modify_ldt");

        asm volatile ("movw %0, %%es" :: "rm" ((unsigned short)7));
        write(1, buf, len);
        return 0;
}

and running ldt_echo >/sys/power/mem

Without the fix, the latter causes a triple fault on resume.

Reported-by: Jarkko Nikula <jarkko.nik...@linux.intel.com>
Fixes: ca37e57bbe0c ("x86/entry/64: Add missing irqflags tracing to 
native_load_gs_index()")
Signed-off-by: Andy Lutomirski <l...@kernel.org>
---

Jarkko, can you test this version?

 arch/x86/power/cpu.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c
index 84fcfde53f8f..5191de14f4df 100644
--- a/arch/x86/power/cpu.c
+++ b/arch/x86/power/cpu.c
@@ -226,8 +226,20 @@ static void notrace __restore_processor_state(struct 
saved_context *ctxt)
        load_idt((const struct desc_ptr *)&ctxt->idt_limit);
 #endif
 
+#ifdef CONFIG_X86_64
        /*
-        * segment registers
+        * We need GSBASE restored before percpu access can work.
+        * percpu access can happen in exception handlers or in complicated
+        * helpers like load_gs_index().
+        */
+       wrmsrl(MSR_GS_BASE, ctxt->gs_base);
+#endif
+
+       fix_processor_context();
+
+       /*
+        * Restore segment registers.  This happens after restoring the GDT
+        * and LDT, which happen in fix_processor_context().
         */
 #ifdef CONFIG_X86_32
        loadsegment(es, ctxt->es);
@@ -248,13 +260,14 @@ static void notrace __restore_processor_state(struct 
saved_context *ctxt)
        load_gs_index(ctxt->gs);
        asm volatile ("movw %0, %%ss" :: "r" (ctxt->ss));
 
+       /*
+        * Restore FSBASE and user GSBASE after reloading the respective
+        * segment selectors.
+        */
        wrmsrl(MSR_FS_BASE, ctxt->fs_base);
-       wrmsrl(MSR_GS_BASE, ctxt->gs_base);
        wrmsrl(MSR_KERNEL_GS_BASE, ctxt->gs_kernel_base);
 #endif
 
-       fix_processor_context();
-
        do_fpu_end();
        tsc_verify_tsc_adjust(true);
        x86_platform.restore_sched_clock_state();
-- 
2.13.6

Reply via email to