Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 6 Jun 2019 16:03:25 +0000 (UTC)
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r348742 - in head/sys: amd64/amd64 cddl/contrib/opensolaris/uts/common/dtrace i386/i386
Message-ID:  <201906061603.x56G3PjT066327@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: markj
Date: Thu Jun  6 16:03:25 2019
New Revision: 348742
URL: https://svnweb.freebsd.org/changeset/base/348742

Log:
  Fix a race between fasttrap and the user breakpoint handler.
  
  When disabling the last enabled userspace probe, fasttrap clears the
  function pointers which hook in to the breakpoint handler.  If a traced
  thread hit a fasttrap breakpoint before it was removed, we must ensure
  that it is able to call the hook; otherwise fasttrap will not consume
  the trap and SIGTRAP will be delievered to the thread.  Synchronize
  with such threads by ensuring that they load the hook pointer with
  interrupts disabled, and by completing an SMP rendezvous after removing
  breakpoints and before clearing the pointers.
  
  Reported by:	Alexander Alexeev <Alexander.Alexeev@dell.com>
  Tested by:	Alexander Alexeev (earlier version)
  Reviewed by:	cem, kib
  MFC after:	1 week
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D20526

Modified:
  head/sys/amd64/amd64/trap.c
  head/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c
  head/sys/i386/i386/trap.c

Modified: head/sys/amd64/amd64/trap.c
==============================================================================
--- head/sys/amd64/amd64/trap.c	Thu Jun  6 15:21:36 2019	(r348741)
+++ head/sys/amd64/amd64/trap.c	Thu Jun  6 16:03:25 2019	(r348742)
@@ -113,6 +113,10 @@ void dblfault_handler(struct trapframe *frame);
 
 static int trap_pfault(struct trapframe *, int);
 static void trap_fatal(struct trapframe *, vm_offset_t);
+#ifdef KDTRACE_HOOKS
+static bool trap_user_dtrace(struct trapframe *,
+    int (**hook)(struct trapframe *));
+#endif
 
 #define MAX_TRAP_MSG		32
 static char *trap_msg[] = {
@@ -284,11 +288,11 @@ trap(struct trapframe *frame)
 			break;
 
 		case T_BPTFLT:		/* bpt instruction fault */
-			enable_intr();
 #ifdef KDTRACE_HOOKS
-			if (dtrace_pid_probe_ptr != NULL &&
-			    dtrace_pid_probe_ptr(frame) == 0)
+			if (trap_user_dtrace(frame, &dtrace_pid_probe_ptr))
 				return;
+#else
+			enable_intr();
 #endif
 			signo = SIGTRAP;
 			ucode = TRAP_BRKPT;
@@ -425,9 +429,7 @@ trap(struct trapframe *frame)
 			break;
 #ifdef KDTRACE_HOOKS
 		case T_DTRACE_RET:
-			enable_intr();
-			if (dtrace_return_probe_ptr != NULL)
-				dtrace_return_probe_ptr(frame);
+			(void)trap_user_dtrace(frame, &dtrace_return_probe_ptr);
 			return;
 #endif
 		}
@@ -947,6 +949,25 @@ trap_fatal(frame, eva)
 	else
 		panic("unknown/reserved trap");
 }
+
+#ifdef KDTRACE_HOOKS
+/*
+ * Invoke a userspace DTrace hook.  The hook pointer is cleared when no
+ * userspace probes are enabled, so we must synchronize with DTrace to ensure
+ * that a trapping thread is able to call the hook before it is cleared.
+ */
+static bool
+trap_user_dtrace(struct trapframe *frame, int (**hookp)(struct trapframe *))
+{
+	int (*hook)(struct trapframe *);
+
+	hook = (int (*)(struct trapframe *))atomic_load_ptr(hookp);
+	enable_intr();
+	if (hook != NULL)
+		return ((hook)(frame) == 0);
+	return (false);
+}
+#endif
 
 /*
  * Double fault handler. Called when a fault occurs while writing

Modified: head/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c	Thu Jun  6 15:21:36 2019	(r348741)
+++ head/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c	Thu Jun  6 16:03:25 2019	(r348742)
@@ -1125,31 +1125,17 @@ fasttrap_enable_callbacks(void)
 static void
 fasttrap_disable_callbacks(void)
 {
-#ifdef illumos
-	ASSERT(MUTEX_HELD(&cpu_lock));
-#endif
-
-
 	mutex_enter(&fasttrap_count_mtx);
 	ASSERT(fasttrap_pid_count > 0);
 	fasttrap_pid_count--;
 	if (fasttrap_pid_count == 0) {
-#ifdef illumos
-		cpu_t *cur, *cpu = CPU;
-
-		for (cur = cpu->cpu_next_onln; cur != cpu;
-		    cur = cur->cpu_next_onln) {
-			rw_enter(&cur->cpu_ft_lock, RW_WRITER);
-		}
-#endif
+		/*
+		 * Synchronize with the breakpoint handler, which is careful to
+		 * enable interrupts only after loading the hook pointer.
+		 */
+		dtrace_sync();
 		dtrace_pid_probe_ptr = NULL;
 		dtrace_return_probe_ptr = NULL;
-#ifdef illumos
-		for (cur = cpu->cpu_next_onln; cur != cpu;
-		    cur = cur->cpu_next_onln) {
-			rw_exit(&cur->cpu_ft_lock);
-		}
-#endif
 	}
 	mutex_exit(&fasttrap_count_mtx);
 }

Modified: head/sys/i386/i386/trap.c
==============================================================================
--- head/sys/i386/i386/trap.c	Thu Jun  6 15:21:36 2019	(r348741)
+++ head/sys/i386/i386/trap.c	Thu Jun  6 16:03:25 2019	(r348742)
@@ -116,6 +116,10 @@ void syscall(struct trapframe *frame);
 
 static int trap_pfault(struct trapframe *, int, vm_offset_t);
 static void trap_fatal(struct trapframe *, vm_offset_t);
+#ifdef KDTRACE_HOOKS
+static bool trap_user_dtrace(struct trapframe *,
+    int (**hook)(struct trapframe *));
+#endif
 void dblfault_handler(void);
 
 extern inthand_t IDTVEC(bpt), IDTVEC(dbg), IDTVEC(int0x80_syscall);
@@ -322,11 +326,11 @@ trap(struct trapframe *frame)
 			break;
 
 		case T_BPTFLT:		/* bpt instruction fault */
-			enable_intr();
 #ifdef KDTRACE_HOOKS
-			if (dtrace_pid_probe_ptr != NULL &&
-			    dtrace_pid_probe_ptr(frame) == 0)
+			if (trap_user_dtrace(frame, &dtrace_pid_probe_ptr))
 				return;
+#else
+			enable_intr();
 #endif
 			signo = SIGTRAP;
 			ucode = TRAP_BRKPT;
@@ -504,9 +508,7 @@ user_trctrap_out:
 			break;
 #ifdef KDTRACE_HOOKS
 		case T_DTRACE_RET:
-			enable_intr();
-			if (dtrace_return_probe_ptr != NULL)
-				dtrace_return_probe_ptr(frame);
+			(void)trap_user_dtrace(frame, &dtrace_return_probe_ptr);
 			return;
 #endif
 		}
@@ -990,6 +992,25 @@ trap_fatal(frame, eva)
 	else
 		panic("unknown/reserved trap");
 }
+
+#ifdef KDTRACE_HOOKS
+/*
+ * Invoke a userspace DTrace hook.  The hook pointer is cleared when no
+ * userspace probes are enabled, so we must synchronize with DTrace to ensure
+ * that a trapping thread is able to call the hook before it is cleared.
+ */
+static bool
+trap_user_dtrace(struct trapframe *frame, int (**hookp)(struct trapframe *))
+{
+	int (*hook)(struct trapframe *);
+
+	hook = (int (*)(struct trapframe *))atomic_load_ptr(hookp);
+	enable_intr();
+	if (hook != NULL)
+		return ((hook)(frame) == 0);
+	return (false);
+}
+#endif
 
 /*
  * Double fault handler. Called when a fault occurs while writing



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201906061603.x56G3PjT066327>