Date: Wed, 24 Jan 2018 21:48:39 +0000 (UTC) From: John Baldwin <jhb@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r328379 - in stable: 10/sys/kern 10/sys/sys 10/tests/sys/kern 11/sys/kern 11/sys/sys 11/tests/sys/kern Message-ID: <201801242148.w0OLmdTr083524@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jhb Date: Wed Jan 24 21:48:39 2018 New Revision: 328379 URL: https://svnweb.freebsd.org/changeset/base/328379 Log: MFC 325028,328344: Discard the correct thread event reported for a ptrace stop. 325028: Discard the correct thread event reported for a ptrace stop. When multiple threads wish to report a tracing event to a debugger, both threads call ptracestop() and one thread will win the race to be the reporting thread (p->p_xthread). The debugger uses PT_LWPINFO with the process ID to determine which thread / LWP is reporting an event and the details of that event. This event is cleared as a side effect of the subsequent ptrace event that resumed the process (PT_CONTINUE, PT_STEP, etc.). However, ptrace() was clearing the event identified by the LWP ID passed to the resume request even if that wasn't the 'p_xthread'. This could result in clearing an event that had not yet been observed by the debugger and leaving the existing event for 'p_thread' pending so that it was reported a second time. Specifically, if the debugger stopped due to a software breakpoint in one thread, but then switched to another thread that was used to resume (e.g. if the user switched to a different thread and issued a step), the resume request (PT_STEP) cleared a pending event (if any) for the thread being stepped. However, the process immediately stopped and the first thread reported it's breakpoint event a second time. The debugger decremented the PC for "both" breakpoint events which resulted in the PC now pointing into the middle of an instruction (on x86) and a SIGILL fault when the process was resumed a second time. To fix, always clear the pending event for 'p_xthread' when resuming a process. ptrace() still honors the requested LWP ID when enabling single-stepping (PT_STEP) or setting a different PC (PT_CONTINUE). 328344: Mark the unused argument to continue_thread() as such. clang in HEAD and 11 does not warn about this, but clang in 10 does. Modified: stable/10/sys/kern/sys_process.c stable/10/sys/sys/param.h stable/10/tests/sys/kern/ptrace_test.c Directory Properties: stable/10/ (props changed) Changes in other areas also in this revision: Modified: stable/11/sys/kern/sys_process.c stable/11/sys/sys/param.h stable/11/tests/sys/kern/ptrace_test.c Directory Properties: stable/11/ (props changed) Modified: stable/10/sys/kern/sys_process.c ============================================================================== --- stable/10/sys/kern/sys_process.c Wed Jan 24 21:39:40 2018 (r328378) +++ stable/10/sys/kern/sys_process.c Wed Jan 24 21:48:39 2018 (r328379) @@ -1072,6 +1072,13 @@ kern_ptrace(struct thread *td, int req, pid_t pid, voi } sendsig: + /* + * Clear the pending event for the thread that just + * reported its event (p_xthread). This may not be + * the thread passed to PT_CONTINUE, PT_STEP, etc. if + * the debugger is resuming a different thread. + */ + td2 = p->p_xthread; if (proctree_locked) { sx_xunlock(&proctree_lock); proctree_locked = 0; Modified: stable/10/sys/sys/param.h ============================================================================== --- stable/10/sys/sys/param.h Wed Jan 24 21:39:40 2018 (r328378) +++ stable/10/sys/sys/param.h Wed Jan 24 21:48:39 2018 (r328379) @@ -58,7 +58,7 @@ * in the range 5 to 9. */ #undef __FreeBSD_version -#define __FreeBSD_version 1004500 /* Master, propagated to newvers */ +#define __FreeBSD_version 1004501 /* Master, propagated to newvers */ /* * __FreeBSD_kernel__ indicates that this system uses the kernel of FreeBSD, Modified: stable/10/tests/sys/kern/ptrace_test.c ============================================================================== --- stable/10/tests/sys/kern/ptrace_test.c Wed Jan 24 21:39:40 2018 (r328378) +++ stable/10/tests/sys/kern/ptrace_test.c Wed Jan 24 21:48:39 2018 (r328379) @@ -2944,6 +2944,174 @@ ATF_TC_BODY(ptrace__event_mask_sigkill_discard, tc) ATF_REQUIRE(errno == ECHILD); } +#if defined(__amd64__) || defined(__i386__) +/* + * Only x86 both define breakpoint() and have a PC after breakpoint so + * that restarting doesn't retrigger the breakpoint. + */ +static void * +continue_thread(void *arg __unused) +{ + breakpoint(); + return (NULL); +} + +static __dead2 void +continue_thread_main(void) +{ + pthread_t threads[2]; + + CHILD_REQUIRE(pthread_create(&threads[0], NULL, continue_thread, + NULL) == 0); + CHILD_REQUIRE(pthread_create(&threads[1], NULL, continue_thread, + NULL) == 0); + CHILD_REQUIRE(pthread_join(threads[0], NULL) == 0); + CHILD_REQUIRE(pthread_join(threads[1], NULL) == 0); + exit(1); +} + +/* + * Ensure that PT_CONTINUE clears the status of the thread that + * triggered the stop even if a different thread's LWP was passed to + * PT_CONTINUE. + */ +ATF_TC_WITHOUT_HEAD(ptrace__PT_CONTINUE_different_thread); +ATF_TC_BODY(ptrace__PT_CONTINUE_different_thread, tc) +{ + struct ptrace_lwpinfo pl; + pid_t fpid, wpid; + lwpid_t lwps[2]; + bool hit_break[2]; + int i, j, status; + + ATF_REQUIRE((fpid = fork()) != -1); + if (fpid == 0) { + trace_me(); + continue_thread_main(); + } + + /* The first wait() should report the stop from SIGSTOP. */ + wpid = waitpid(fpid, &status, 0); + ATF_REQUIRE(wpid == fpid); + ATF_REQUIRE(WIFSTOPPED(status)); + ATF_REQUIRE(WSTOPSIG(status) == SIGSTOP); + + ATF_REQUIRE(ptrace(PT_LWPINFO, wpid, (caddr_t)&pl, + sizeof(pl)) != -1); + + ATF_REQUIRE(ptrace(PT_LWP_EVENTS, wpid, NULL, 1) == 0); + + /* Continue the child ignoring the SIGSTOP. */ + ATF_REQUIRE(ptrace(PT_CONTINUE, fpid, (caddr_t)1, 0) == 0); + + /* One of the new threads should report it's birth. */ + wpid = waitpid(fpid, &status, 0); + ATF_REQUIRE(wpid == fpid); + ATF_REQUIRE(WIFSTOPPED(status)); + ATF_REQUIRE(WSTOPSIG(status) == SIGTRAP); + + ATF_REQUIRE(ptrace(PT_LWPINFO, wpid, (caddr_t)&pl, sizeof(pl)) != -1); + ATF_REQUIRE((pl.pl_flags & (PL_FLAG_BORN | PL_FLAG_SCX)) == + (PL_FLAG_BORN | PL_FLAG_SCX)); + lwps[0] = pl.pl_lwpid; + + /* + * Suspend this thread to ensure both threads are alive before + * hitting the breakpoint. + */ + ATF_REQUIRE(ptrace(PT_SUSPEND, lwps[0], NULL, 0) != -1); + + ATF_REQUIRE(ptrace(PT_CONTINUE, fpid, (caddr_t)1, 0) == 0); + + /* Second thread should report it's birth. */ + wpid = waitpid(fpid, &status, 0); + ATF_REQUIRE(wpid == fpid); + ATF_REQUIRE(WIFSTOPPED(status)); + ATF_REQUIRE(WSTOPSIG(status) == SIGTRAP); + + ATF_REQUIRE(ptrace(PT_LWPINFO, wpid, (caddr_t)&pl, sizeof(pl)) != -1); + ATF_REQUIRE((pl.pl_flags & (PL_FLAG_BORN | PL_FLAG_SCX)) == + (PL_FLAG_BORN | PL_FLAG_SCX)); + ATF_REQUIRE(pl.pl_lwpid != lwps[0]); + lwps[1] = pl.pl_lwpid; + + /* Resume both threads waiting for breakpoint events. */ + hit_break[0] = hit_break[1] = false; + ATF_REQUIRE(ptrace(PT_RESUME, lwps[0], NULL, 0) != -1); + ATF_REQUIRE(ptrace(PT_CONTINUE, fpid, (caddr_t)1, 0) == 0); + + /* One thread should report a breakpoint. */ + wpid = waitpid(fpid, &status, 0); + ATF_REQUIRE(wpid == fpid); + ATF_REQUIRE(WIFSTOPPED(status)); + ATF_REQUIRE(WSTOPSIG(status) == SIGTRAP); + + ATF_REQUIRE(ptrace(PT_LWPINFO, wpid, (caddr_t)&pl, sizeof(pl)) != -1); + ATF_REQUIRE((pl.pl_flags & PL_FLAG_SI) != 0); + ATF_REQUIRE(pl.pl_siginfo.si_signo == SIGTRAP && + pl.pl_siginfo.si_code == TRAP_BRKPT); + if (pl.pl_lwpid == lwps[0]) + i = 0; + else + i = 1; + hit_break[i] = true; + + /* + * Resume both threads but pass the other thread's LWPID to + * PT_CONTINUE. + */ + ATF_REQUIRE(ptrace(PT_CONTINUE, lwps[i ^ 1], (caddr_t)1, 0) == 0); + + /* + * Will now get two thread exit events and one more breakpoint + * event. + */ + for (j = 0; j < 3; j++) { + wpid = waitpid(fpid, &status, 0); + ATF_REQUIRE(wpid == fpid); + ATF_REQUIRE(WIFSTOPPED(status)); + ATF_REQUIRE(WSTOPSIG(status) == SIGTRAP); + + ATF_REQUIRE(ptrace(PT_LWPINFO, wpid, (caddr_t)&pl, + sizeof(pl)) != -1); + + if (pl.pl_lwpid == lwps[0]) + i = 0; + else + i = 1; + + ATF_REQUIRE_MSG(lwps[i] != 0, "event for exited thread"); + if (pl.pl_flags & PL_FLAG_EXITED) { + ATF_REQUIRE_MSG(hit_break[i], + "exited thread did not report breakpoint"); + lwps[i] = 0; + } else { + ATF_REQUIRE((pl.pl_flags & PL_FLAG_SI) != 0); + ATF_REQUIRE(pl.pl_siginfo.si_signo == SIGTRAP && + pl.pl_siginfo.si_code == TRAP_BRKPT); + ATF_REQUIRE_MSG(!hit_break[i], + "double breakpoint event"); + hit_break[i] = true; + } + + ATF_REQUIRE(ptrace(PT_CONTINUE, fpid, (caddr_t)1, 0) == 0); + } + + /* Both threads should have exited. */ + ATF_REQUIRE(lwps[0] == 0); + ATF_REQUIRE(lwps[1] == 0); + + /* The last event should be for the child process's exit. */ + wpid = waitpid(fpid, &status, 0); + ATF_REQUIRE(WIFEXITED(status)); + ATF_REQUIRE(WEXITSTATUS(status) == 1); + + wpid = wait(&status); + ATF_REQUIRE(wpid == -1); + ATF_REQUIRE(errno == ECHILD); +} +#endif + ATF_TP_ADD_TCS(tp) { @@ -2990,6 +3158,9 @@ ATF_TP_ADD_TCS(tp) ATF_TP_ADD_TC(tp, ptrace__parent_terminate_with_pending_sigstop1); ATF_TP_ADD_TC(tp, ptrace__parent_terminate_with_pending_sigstop2); ATF_TP_ADD_TC(tp, ptrace__event_mask_sigkill_discard); +#if defined(__amd64__) || defined(__i386__) + ATF_TP_ADD_TC(tp, ptrace__PT_CONTINUE_different_thread); +#endif return (atf_no_error()); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201801242148.w0OLmdTr083524>