x86/arch_prctl/64: restore accidentally removed put_cpu in ARCH_SET_GS

This fixes 731e33e39a5b95ad770 "Remove FSBASE/GSBASE < 4G optimization"

Signed-off-by Mateusz Guzik <[email protected]>

---
 arch/x86/kernel/process_64.c | 1 +
 1 file changed, 1 insertion(+)

arch/x86/kernel/process_64.c +1 -1

index 4285f6a..6b16c36 100644
@@ -541,6 +541,7 @@ long do_arch_prctl(struct task_struct *task, int code, unsigned long addr)
                        load_gs_index(0);
                        ret = wrmsrl_safe(MSR_KERNEL_GS_BASE, addr);
                }
+               put_cpu();
                break;
        case ARCH_SET_FS:
                /* Not strictly needed for fs, but do it for symmetry
--

1.8.3.1

Quoted 1 line · Mateusz Guzik

This fixes 731e33e39a5b95ad770 "Remove FSBASE/GSBASE < 4G optimization"

Indeed. How did that survive lockdep?

Quoted 5 lines · Andy Lutomirski

Quoted 1 line · Mateusz Guzik

This fixes 731e33e39a5b95ad770 "Remove FSBASE/GSBASE < 4G optimization"

Indeed. How did that survive lockdep?

lockdep_sys_exit only checks actual locks.

In the common path after return from particular syscall interrupts get blindly disabled (as opposed to checking first that they are enabled). preemption count is not checked in the fast path at all and is checked elsewhere as a side effect of calls to e.g. schedule().

How about a hack along these lines (note I don't claim this is committable as it is, but it should work):

arch/x86/entry/common.c +19 -3

index ec138e5..5887bc7 100644
@@ -303,6 +303,24 @@ static void syscall_slow_exit_work(struct pt_regs *regs, u32 cached_flags)
                tracehook_report_syscall_exit(regs, step);
 }
 
+#ifdef CONFIG_PROVE_LOCKING
+/*
+ * Called after syscall handlers return.
+ */
+__visible void syscall_assert_exit(struct pt_regs *regs)
+{
+       if (in_atomic() || irqs_disabled()) {
+               printk(KERN_ERR "invalid state on exit from syscall %ld: "
+                       "in_atomic(): %d, irqs_disabled(): %d, pid: %d, "
+                       "name: %s\n", regs->orig_ax, in_atomic(),
+                       irqs_disabled(), current->pid, current->comm);
+       }
+
+       if (irqs_disabled())
+               local_irq_enable();
+}
+#endif
+
 /*
  * Called with IRQs on and fully valid regs.  Returns with IRQs off in a
  * state such that we can immediately switch to user mode.

@@ -314,9 +332,7 @@ __visible inline void syscall_return_slowpath(struct pt_regs *regs)
 
        CT_WARN_ON(ct_state() != CONTEXT_KERNEL);
 
-       if (IS_ENABLED(CONFIG_PROVE_LOCKING) &&
-           WARN(irqs_disabled(), "syscall %ld left IRQs disabled", regs->orig_ax))
-               local_irq_enable();
+       syscall_assert_exit(regs);
 
        /*
         * First do one-time work.  If these work items are enabled, we

arch/x86/entry/entry_64.S +6 -0

index 9ee0da1..6c5cc23 100644
@@ -210,6 +210,12 @@ entry_SYSCALL_64_fastpath:
        movq    %rax, RAX(%rsp)
 1:
 
+#ifdef CONFIG_PROVE_LOCKING
+       /*
+        * We want to validate bunch of stuff, which will clobber registers.
+        */
+       jmp     2f
+#endif
        /*
         * If we get here, then we know that pt_regs is clean for SYSRET64.
         * If we see that no exit work is required (which we are required

--
Mateusz Guzik

This fixes 731e33e39a5b95ad770 "Remove FSBASE/GSBASE < 4G optimization"

Signed-off-by Mateusz Guzik <[email protected]>

---
 arch/x86/kernel/process_64.c | 1 +
 1 file changed, 1 insertion(+)

arch/x86/kernel/process_64.c +1 -1

index 4285f6a..6b16c36 100644
@@ -541,6 +541,7 @@ long do_arch_prctl(struct task_struct *task, int code, unsigned long addr)
                        load_gs_index(0);
                        ret = wrmsrl_safe(MSR_KERNEL_GS_BASE, addr);
                }
+               put_cpu();
                break;
        case ARCH_SET_FS:
                /* Not strictly needed for fs, but do it for symmetry
--

1.8.3.1

Quoted 16 lines · Mateusz Guzik

This fixes 731e33e39a5b95ad770 "Remove FSBASE/GSBASE < 4G optimization"

Signed-off-by Mateusz Guzik <[email protected]>

---
 arch/x86/kernel/process_64.c | 1 +
 1 file changed, 1 insertion(+)

arch/x86/kernel/process_64.c +1 -0

index 4285f6a..6b16c36 100644
@@ -541,6 +541,7 @@ long do_arch_prctl(struct task_struct *task, int code, unsigned long addr)
                        load_gs_index(0);
                        ret = wrmsrl_safe(MSR_KERNEL_GS_BASE, addr);
                }
+               put_cpu();

Quoted 6 lines

arch/x86/kernel/process_64.c +0 -1

                break;
        case ARCH_SET_FS:
                /* Not strictly needed for fs, but do it for symmetry
--

1.8.3.1

Ingo, can you apply this before the merge window opens?

I just noticed that you weren't cc'd, so I'll repeat my ack:

Acked-by Andy Lutomirski <[email protected]>

And I'll ask, since IIRC you wrote it: would it make sense to augment lockdep_sys_exit to see if preemption got left disabled?