Skip to content
Permalink
Browse files

Move TRAP_CHLD/TRAP_LWP ptrace information from struct proc to siginfo

Storing struct ptrace_state information inside struct proc was vulnerable
to synchronization bugs, as multiple events emitted in the same time were
overwritting other ones.

Cache the original parent process id in p_oppid. Reusing here p_opptr is
in theory prone to slight race codition.

Change the semantics of PT_GET_PROCESS_STATE, reutning EINVAL for calls
prompting for the value in cases when there wasn't registered an
appropriate event.

Add an alternative approach to check the ptrace_state information, directly
from the siginfo_t value returned from PT_GET_SIGINFO. The original
PT_GET_PROCESS_STATE approach is kept for compat with older NetBSD and
OpenBSD. New code is recommended to keep using PT_GET_PROCESS_STATE.

Add a couple of compile-time asserts for assumptions in the code.

No functional change intended in existing ptrace(2) software.

All ATF ptrace(2) and ATF GDB tests pass.

This change improves reliability of the threading ptrace(2) code.
  • Loading branch information...
krytarowski committed Sep 30, 2019
1 parent 936d736 commit b61dd2f758a98b11e97cd8287de9d800b04f2f5d
@@ -1,4 +1,4 @@
/* $NetBSD: siginfo.h,v 1.7 2019/06/30 08:49:21 martin Exp $ */
/* $NetBSD: siginfo.h,v 1.8 2019/09/30 21:13:33 kamil Exp $ */

/*-
* Copyright (c) 2002 The NetBSD Foundation, Inc.
@@ -75,6 +75,14 @@ struct __ksiginfo32 {
int _error;
uint64_t _args[8]; /* SYS_MAXSYSARGS */
} _syscall;

struct {
int _pe_report_event;
union {
pid_t _pe_other_pid;
lwpid_t _pe_lwp;
} _option;
} _ptrace_state;
} _reason;
};

@@ -1,4 +1,4 @@
/* $NetBSD: kern_exec.c,v 1.481 2019/09/17 15:19:27 christos Exp $ */
/* $NetBSD: kern_exec.c,v 1.482 2019/09/30 21:13:33 kamil Exp $ */

/*-
* Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -59,7 +59,7 @@
*/

#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: kern_exec.c,v 1.481 2019/09/17 15:19:27 christos Exp $");
__KERNEL_RCSID(0, "$NetBSD: kern_exec.c,v 1.482 2019/09/30 21:13:33 kamil Exp $");

#include "opt_exec.h"
#include "opt_execfmt.h"
@@ -1339,7 +1339,7 @@ execve_runproc(struct lwp *l, struct execve_data * restrict data,
/* posix_spawn(3) reports a single event with implied exec(3) */
if ((p->p_slflag & PSL_TRACED) && !is_spawn) {
mutex_enter(p->p_lock);
eventswitch(TRAP_EXEC);
eventswitch(TRAP_EXEC, 0, 0);
mutex_enter(proc_lock);
}

@@ -2249,7 +2249,7 @@ spawn_return(void *arg)
}

mutex_enter(p->p_lock);
eventswitch(TRAP_CHLD);
eventswitch(TRAP_CHLD, PTRACE_POSIX_SPAWN, p->p_oppid);
}

cpu_return:
@@ -2630,8 +2630,7 @@ do_posix_spawn(struct lwp *l1, pid_t *pid_res, bool *child_ok, const char *path,
if ((p1->p_slflag & (PSL_TRACEPOSIX_SPAWN|PSL_TRACED)) ==
(PSL_TRACEPOSIX_SPAWN|PSL_TRACED)) {
proc_changeparent(p2, p1->p_pptr);
p1->p_pspid = p2->p_pid;
p2->p_pspid = p1->p_pid;
p2->p_oppid = p1->p_pid;
}

LIST_INSERT_AFTER(p1, p2, p_pglist);
@@ -2685,7 +2684,7 @@ do_posix_spawn(struct lwp *l1, pid_t *pid_res, bool *child_ok, const char *path,
}

mutex_enter(p1->p_lock);
eventswitch(TRAP_CHLD);
eventswitch(TRAP_CHLD, PTRACE_POSIX_SPAWN, pid);
}
return 0;

@@ -1,4 +1,4 @@
/* $NetBSD: kern_fork.c,v 1.213 2019/06/13 20:20:18 kamil Exp $ */
/* $NetBSD: kern_fork.c,v 1.214 2019/09/30 21:13:33 kamil Exp $ */

/*-
* Copyright (c) 1999, 2001, 2004, 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -67,7 +67,7 @@
*/

#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: kern_fork.c,v 1.213 2019/06/13 20:20:18 kamil Exp $");
__KERNEL_RCSID(0, "$NetBSD: kern_fork.c,v 1.214 2019/09/30 21:13:33 kamil Exp $");

#include "opt_ktrace.h"
#include "opt_dtrace.h"
@@ -252,7 +252,6 @@ fork1(struct lwp *l1, int flags, int exitsig, void *stack, size_t stacksize,
int count;
vaddr_t uaddr;
int tnprocs;
bool trace_fork, trace_vfork;
int error = 0;

p1 = l1->l_proc;
@@ -511,17 +510,9 @@ fork1(struct lwp *l1, int flags, int exitsig, void *stack, size_t stacksize,
/*
* Trace fork(2) and vfork(2)-like events on demand in a debugger.
*/
trace_fork = tracefork(p1, flags);
trace_vfork = tracevfork(p1, flags);
if (trace_fork || trace_vfork)
if (tracefork(p1, flags) || tracevfork(p1, flags)) {
proc_changeparent(p2, p1->p_pptr);
if (trace_fork) {
p1->p_fpid = p2->p_pid;
p2->p_fpid = p1->p_pid;
}
if (trace_vfork) {
p1->p_vfpid = p2->p_pid;
p2->p_vfpid = p1->p_pid;
p2->p_oppid = p1->p_pid;
}

LIST_INSERT_AFTER(p1, p2, p_pglist);
@@ -605,7 +596,9 @@ fork1(struct lwp *l1, int flags, int exitsig, void *stack, size_t stacksize,
*/
if (tracefork(p1, flags) || tracevfork(p1, flags)) {
mutex_enter(p1->p_lock);
eventswitch(TRAP_CHLD);
eventswitch(TRAP_CHLD,
tracefork(p1, flags) ? PTRACE_FORK : PTRACE_VFORK,
retval[0]);
mutex_enter(proc_lock);
}

@@ -621,8 +614,7 @@ fork1(struct lwp *l1, int flags, int exitsig, void *stack, size_t stacksize,
*/
if (tracevforkdone(p1, flags)) {
mutex_enter(p1->p_lock);
p1->p_vfpid_done = retval[0];
eventswitch(TRAP_CHLD);
eventswitch(TRAP_CHLD, PTRACE_VFORK_DONE, retval[0]);
} else
mutex_exit(proc_lock);

@@ -645,9 +637,10 @@ child_return(void *arg)
mutex_exit(proc_lock);
goto my_tracer_is_gone;
}

mutex_enter(p->p_lock);
eventswitch(TRAP_CHLD);
eventswitch(TRAP_CHLD,
ISSET(p->p_lflag, PL_PPWAIT) ? PTRACE_VFORK : PTRACE_FORK,
p->p_oppid);
}

my_tracer_is_gone:
@@ -1,4 +1,4 @@
/* $NetBSD: kern_lwp.c,v 1.202 2019/06/04 11:54:03 kamil Exp $ */
/* $NetBSD: kern_lwp.c,v 1.203 2019/09/30 21:13:33 kamil Exp $ */

/*-
* Copyright (c) 2001, 2006, 2007, 2008, 2009 The NetBSD Foundation, Inc.
@@ -211,7 +211,7 @@
*/

#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: kern_lwp.c,v 1.202 2019/06/04 11:54:03 kamil Exp $");
__KERNEL_RCSID(0, "$NetBSD: kern_lwp.c,v 1.203 2019/09/30 21:13:33 kamil Exp $");

#include "opt_ddb.h"
#include "opt_lockdebug.h"
@@ -239,6 +239,7 @@ __KERNEL_RCSID(0, "$NetBSD: kern_lwp.c,v 1.202 2019/06/04 11:54:03 kamil Exp $")
#include <sys/fstrans.h>
#include <sys/dtrace_bsd.h>
#include <sys/sdt.h>
#include <sys/ptrace.h>
#include <sys/xcall.h>
#include <sys/uidinfo.h>
#include <sys/sysctl.h>
@@ -1091,8 +1092,7 @@ lwp_exit(struct lwp *l)
* about a terminating LWP as it would deadlock.
*/
} else {
p->p_lwp_exited = l->l_lid;
eventswitch(TRAP_LWP);
eventswitch(TRAP_LWP, PTRACE_LWP_EXIT, l->l_lid);
mutex_enter(proc_lock);
}
}
@@ -1,4 +1,4 @@
/* $NetBSD: kern_proc.c,v 1.234 2019/08/02 22:46:44 kamil Exp $ */
/* $NetBSD: kern_proc.c,v 1.235 2019/09/30 21:13:33 kamil Exp $ */

/*-
* Copyright (c) 1999, 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -62,7 +62,7 @@
*/

#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: kern_proc.c,v 1.234 2019/08/02 22:46:44 kamil Exp $");
__KERNEL_RCSID(0, "$NetBSD: kern_proc.c,v 1.235 2019/09/30 21:13:33 kamil Exp $");

#ifdef _KERNEL_OPT
#include "opt_kstack.h"
@@ -2263,12 +2263,7 @@ fill_proc(const struct proc *psrc, struct proc *p, bool allowaddr)
COND_SET_VALUE(p->p_sigpend, psrc->p_sigpend, allowaddr);
COND_SET_VALUE(p->p_lwpctl, psrc->p_lwpctl, allowaddr);
p->p_ppid = psrc->p_ppid;
p->p_fpid = psrc->p_fpid;
p->p_vfpid = psrc->p_vfpid;
p->p_vfpid_done = psrc->p_vfpid_done;
p->p_lwp_created = psrc->p_lwp_created;
p->p_lwp_exited = psrc->p_lwp_exited;
p->p_pspid = psrc->p_pspid;
p->p_oppid = psrc->p_oppid;
COND_SET_VALUE(p->p_path, psrc->p_path, allowaddr);
COND_SET_VALUE(p->p_sigctx, psrc->p_sigctx, allowaddr);
p->p_nice = psrc->p_nice;
@@ -1,4 +1,4 @@
/* $NetBSD: kern_sig.c,v 1.364 2019/06/21 04:28:12 kamil Exp $ */
/* $NetBSD: kern_sig.c,v 1.365 2019/09/30 21:13:33 kamil Exp $ */

/*-
* Copyright (c) 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -70,7 +70,7 @@
*/

#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: kern_sig.c,v 1.364 2019/06/21 04:28:12 kamil Exp $");
__KERNEL_RCSID(0, "$NetBSD: kern_sig.c,v 1.365 2019/09/30 21:13:33 kamil Exp $");

#include "opt_ptrace.h"
#include "opt_dtrace.h"
@@ -1560,7 +1560,7 @@ proc_stop_done(struct proc *p, int ppmask)
* an event specific to a traced process only.
*/
void
eventswitch(int code)
eventswitch(int code, int pe_report_event, int entity)
{
struct lwp *l = curlwp;
struct proc *p = l->l_proc;
@@ -1605,8 +1605,12 @@ eventswitch(int code)

KSI_INIT_TRAP(&ksi);
ksi.ksi_lid = l->l_lid;
ksi.ksi_info._signo = signo;
ksi.ksi_info._code = code;
ksi.ksi_signo = signo;
ksi.ksi_code = code;
ksi.ksi_pe_report_event = pe_report_event;

CTASSERT(sizeof(ksi.ksi_pe_other_pid) == sizeof(ksi.ksi_pe_lwp));
ksi.ksi_pe_other_pid = entity;

/* Needed for ktrace */
ps = p->p_sigacts;
@@ -1,4 +1,4 @@
/* $NetBSD: sys_lwp.c,v 1.69 2019/07/10 17:52:22 maxv Exp $ */
/* $NetBSD: sys_lwp.c,v 1.70 2019/09/30 21:13:33 kamil Exp $ */

/*-
* Copyright (c) 2001, 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -35,7 +35,7 @@
*/

#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: sys_lwp.c,v 1.69 2019/07/10 17:52:22 maxv Exp $");
__KERNEL_RCSID(0, "$NetBSD: sys_lwp.c,v 1.70 2019/09/30 21:13:33 kamil Exp $");

#include <sys/param.h>
#include <sys/systm.h>
@@ -45,6 +45,7 @@ __KERNEL_RCSID(0, "$NetBSD: sys_lwp.c,v 1.69 2019/07/10 17:52:22 maxv Exp $");
#include <sys/syscallargs.h>
#include <sys/kauth.h>
#include <sys/kmem.h>
#include <sys/ptrace.h>
#include <sys/sleepq.h>
#include <sys/lwpctl.h>
#include <sys/cpu.h>
@@ -91,8 +92,7 @@ mi_startlwp(void *arg)
}

mutex_enter(p->p_lock);
p->p_lwp_created = l->l_lid;
eventswitch(TRAP_LWP);
eventswitch(TRAP_LWP, PTRACE_LWP_CREATE, l->l_lid);
}
}

@@ -1,4 +1,4 @@
/* $NetBSD: sys_ptrace_common.c,v 1.58 2019/07/18 20:10:46 kamil Exp $ */
/* $NetBSD: sys_ptrace_common.c,v 1.59 2019/09/30 21:13:33 kamil Exp $ */

/*-
* Copyright (c) 2008, 2009 The NetBSD Foundation, Inc.
@@ -118,7 +118,7 @@
*/

#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: sys_ptrace_common.c,v 1.58 2019/07/18 20:10:46 kamil Exp $");
__KERNEL_RCSID(0, "$NetBSD: sys_ptrace_common.c,v 1.59 2019/09/30 21:13:33 kamil Exp $");

#ifdef _KERNEL_OPT
#include "opt_ptrace.h"
@@ -694,27 +694,25 @@ ptrace_get_process_state(struct proc *t, void *addr, size_t data)
DPRINTF(("%s: %zu != %zu\n", __func__, data, sizeof(ps)));
return EINVAL;
}
memset(&ps, 0, sizeof(ps));

if (t->p_fpid) {
ps.pe_report_event = PTRACE_FORK;
ps.pe_other_pid = t->p_fpid;
} else if (t->p_vfpid) {
ps.pe_report_event = PTRACE_VFORK;
ps.pe_other_pid = t->p_vfpid;
} else if (t->p_vfpid_done) {
ps.pe_report_event = PTRACE_VFORK_DONE;
ps.pe_other_pid = t->p_vfpid_done;
} else if (t->p_lwp_created) {
ps.pe_report_event = PTRACE_LWP_CREATE;
ps.pe_lwp = t->p_lwp_created;
} else if (t->p_lwp_exited) {
ps.pe_report_event = PTRACE_LWP_EXIT;
ps.pe_lwp = t->p_lwp_exited;
} else if (t->p_pspid) {
ps.pe_report_event = PTRACE_POSIX_SPAWN;
ps.pe_other_pid = t->p_pspid;

if (t->p_sigctx.ps_info._signo != SIGTRAP ||
(t->p_sigctx.ps_info._code != TRAP_CHLD &&
t->p_sigctx.ps_info._code != TRAP_LWP)) {
return EINVAL;
}

ps.pe_report_event =
t->p_sigctx.ps_info._reason._ptrace_state._pe_report_event;

CTASSERT(sizeof(ps.pe_other_pid) ==
sizeof(t->p_sigctx.ps_info._reason._ptrace_state._option._pe_other_pid));
CTASSERT(sizeof(ps.pe_lwp) ==
sizeof(t->p_sigctx.ps_info._reason._ptrace_state._option._pe_other_pid));
CTASSERT(sizeof(ps.pe_other_pid) == sizeof(ps.pe_lwp));

ps.pe_other_pid =
t->p_sigctx.ps_info._reason._ptrace_state._option._pe_other_pid;

DPRINTF(("%s: lwp=%d event=%#x pid=%d lwp=%d\n", __func__,
t->p_sigctx.ps_lwp, ps.pe_report_event,
ps.pe_other_pid, ps.pe_lwp));
@@ -894,13 +892,6 @@ ptrace_sendsig(struct proc *t, struct lwp *lt, int signo, int resume_all)
{
ksiginfo_t ksi;

t->p_fpid = 0;
t->p_vfpid = 0;
t->p_vfpid_done = 0;
t->p_lwp_created = 0;
t->p_lwp_exited = 0;
t->p_pspid = 0;

/* Finally, deliver the requested signal (or none). */
if (t->p_stat == SSTOP) {
/*
@@ -1,4 +1,4 @@
/* $NetBSD: proc.h,v 1.355 2019/07/15 19:34:45 pgoyette Exp $ */
/* $NetBSD: proc.h,v 1.356 2019/09/30 21:13:33 kamil Exp $ */

/*-
* Copyright (c) 2006, 2007, 2008 The NetBSD Foundation, Inc.
@@ -311,12 +311,7 @@ struct proc {
sigpend_t p_sigpend; /* p: pending signals */
struct lcproc *p_lwpctl; /* p, a: _lwp_ctl() information */
pid_t p_ppid; /* :: cached parent pid */
pid_t p_fpid; /* :: forked pid */
pid_t p_vfpid; /* :: vforked pid */
pid_t p_vfpid_done; /* :: vforked done pid */
lwpid_t p_lwp_created; /* :: lwp created */
lwpid_t p_lwp_exited; /* :: lwp exited */
pid_t p_pspid; /* :: posix_spawn pid */
pid_t p_oppid; /* :: cached original parent pid */
char *p_path; /* :: full pathname of executable */

/*

0 comments on commit b61dd2f

Please sign in to comment.
You can’t perform that action at this time.