[1/2] Revert "x86/mm: Only set IBPB when the new thread cannot ptrace current thread"

Message ID 1522907769-14867-2-git-send-email-tyhicks@canonical.com
State New
Headers show
Series
  • Fix deadlock on task switches with new microcode
Related show

Commit Message

Tyler Hicks April 5, 2018, 5:56 a.m.
BugLink: https://bugs.launchpad.net/bugs/175992

This reverts commit 3f21d934ae3580f2b2d3b8095bea02f31e4dc58b.

Using a ptrace access check in the middle of a task switch was causing
a hard lockup in some cases when the old task was confined by AppArmor.
If the AppArmor policy for the the old task didn't allow the task to
ptrace the new task, AppArmor would attempt to emit an audit message and
deadlock on the task's pi_lock would occur. The fix is to revert this
change and go with upstream's implementation that uses the task's
dumpable state to determine if IBPB should be used.

Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
---
 arch/x86/mm/tlb.c      |  5 +----
 include/linux/ptrace.h |  6 ------
 kernel/ptrace.c        | 18 ++++--------------
 3 files changed, 5 insertions(+), 24 deletions(-)

Patch

diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 31bc788..c096254 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -7,7 +7,6 @@ 
 #include <linux/module.h>
 #include <linux/cpu.h>
 #include <linux/debugfs.h>
-#include <linux/ptrace.h>
 
 #include <asm/tlbflush.h>
 #include <asm/mmu_context.h>
@@ -145,9 +144,7 @@  void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
 		/* Stop flush ipis for the previous mm */
 		cpumask_clear_cpu(cpu, mm_cpumask(prev));
 
-		/* Null tsk means switching to kernel, so that's safe */
-		if (ibpb_inuse && tsk &&
-			___ptrace_may_access(tsk, current, PTRACE_MODE_IBPB))
+		if (ibpb_inuse && boot_cpu_has(X86_FEATURE_SPEC_CTRL))
 			native_wrmsrl(MSR_IA32_PRED_CMD, FEATURE_SET_IBPB);
 
 		/* Load per-mm CR4 state */
diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index c84b881..81fdf4b 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -59,15 +59,12 @@  extern void exit_ptrace(struct task_struct *tracer, struct list_head *dead);
 #define PTRACE_MODE_NOAUDIT	0x04
 #define PTRACE_MODE_FSCREDS 0x08
 #define PTRACE_MODE_REALCREDS 0x10
-#define PTRACE_MODE_NOACCESS_CHK 0x20
 
 /* shorthands for READ/ATTACH and FSCREDS/REALCREDS combinations */
 #define PTRACE_MODE_READ_FSCREDS (PTRACE_MODE_READ | PTRACE_MODE_FSCREDS)
 #define PTRACE_MODE_READ_REALCREDS (PTRACE_MODE_READ | PTRACE_MODE_REALCREDS)
 #define PTRACE_MODE_ATTACH_FSCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_FSCREDS)
 #define PTRACE_MODE_ATTACH_REALCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_REALCREDS)
-#define PTRACE_MODE_IBPB (PTRACE_MODE_ATTACH | PTRACE_MODE_NOAUDIT \
-			| PTRACE_MODE_NOACCESS_CHK | PTRACE_MODE_REALCREDS)
 
 /**
  * ptrace_may_access - check whether the caller is permitted to access
@@ -85,9 +82,6 @@  extern void exit_ptrace(struct task_struct *tracer, struct list_head *dead);
  */
 extern bool ptrace_may_access(struct task_struct *task, unsigned int mode);
 
-extern int ___ptrace_may_access(struct task_struct *cur, struct task_struct *task,
-	unsigned int mode);
-
 static inline int ptrace_reparented(struct task_struct *child)
 {
 	return !same_thread_group(child->real_parent, child->parent);
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 14cc49a5..5e2cd10 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -235,10 +235,9 @@  static int ptrace_has_cap(struct user_namespace *ns, unsigned int mode)
 }
 
 /* Returns 0 on success, -errno on denial. */
-int ___ptrace_may_access(struct task_struct *cur, struct task_struct *task,
-		unsigned int mode)
+static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
 {
-	const struct cred *cred = __task_cred(cur), *tcred;
+	const struct cred *cred = current_cred(), *tcred;
 	struct mm_struct *mm;
 	kuid_t caller_uid;
 	kgid_t caller_gid;
@@ -258,7 +257,7 @@  int ___ptrace_may_access(struct task_struct *cur, struct task_struct *task,
 	 */
 
 	/* Don't let security modules deny introspection */
-	if (same_thread_group(task, cur))
+	if (same_thread_group(task, current))
 		return 0;
 	rcu_read_lock();
 	if (mode & PTRACE_MODE_FSCREDS) {
@@ -296,16 +295,7 @@  ok:
 	     !ptrace_has_cap(mm->user_ns, mode)))
 	    return -EPERM;
 
-	if (!(mode & PTRACE_MODE_NOACCESS_CHK))
-		return security_ptrace_access_check(task, mode);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(___ptrace_may_access);
-
-static int __ptrace_may_access(struct task_struct *task, unsigned int mode)
-{
-	return ___ptrace_may_access(current, task, mode);
+	return security_ptrace_access_check(task, mode);
 }
 
 bool ptrace_may_access(struct task_struct *task, unsigned int mode)