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>