1
0
mirror of https://git.FreeBSD.org/src.git synced 2025-01-02 12:20:51 +00:00

Close some races between procfs/ptrace and exit(2):

- Reorder the events in exit(2) slightly so that we trigger the S_EXIT
  stop event earlier.  After we have signalled that, we set P_WEXIT and
  then wait for any processes with a hold on the vmspace via PHOLD to
  release it.  PHOLD now KASSERT()'s that P_WEXIT is clear when it is
  invoked, and PRELE now does a wakeup if P_WEXIT is set and p_lock drops
  to zero.
- Change proc_rwmem() to require that the processing read from has its
  vmspace held via PHOLD by the caller and get rid of all the junk to
  screw around with the vmspace reference count as we no longer need it.
- In ptrace() and pseudofs(), treat a process with P_WEXIT set as if it
  doesn't exist.
- Only do one PHOLD in kern_ptrace() now, and do it earlier so it covers
  FIX_SSTEP() (since on alpha at least this can end up calling proc_rwmem()
  to clear an earlier single-step simualted via a breakpoint).  We only
  do one to avoid races.  Also, by making the EINVAL error for unknown
  requests be part of the default: case in the switch, the various
  switch cases can now just break out to return which removes a _lot_ of
  duplicated PRELE and proc unlocks, etc.  Also, it fixes at least one bug
  where a LWP ptrace command could return EINVAL with the proc lock still
  held.
- Changed the locking for ptrace_single_step(), ptrace_set_pc(), and
  ptrace_clear_single_step() to always be called with the proc lock
  held (it was a mixed bag previously).  Alpha and arm have to drop
  the lock while the mess around with breakpoints, but other archs
  avoid extra lock release/acquires in ptrace().  I did have to fix a
  couple of other consumers in kern_kse and a few other places to
  hold the proc lock and PHOLD.

Tested by:	ps (1 mostly, but some bits of 2-4 as well)
MFC after:	1 week
This commit is contained in:
John Baldwin 2006-02-22 18:57:50 +00:00
parent 57fb5e6097
commit 06ad42b2f7
Notes: svn2git 2020-12-20 02:59:44 +00:00
svn path=/head/; revision=155922
11 changed files with 164 additions and 115 deletions

View File

@ -1756,6 +1756,8 @@ ptrace_read_int(struct thread *td, vm_offset_t addr, u_int32_t *v)
{
struct iovec iov;
struct uio uio;
PROC_LOCK_ASSERT(td->td_proc, MA_NOTOWNED);
iov.iov_base = (caddr_t) v;
iov.iov_len = sizeof(u_int32_t);
uio.uio_iov = &iov;
@ -1773,6 +1775,8 @@ ptrace_write_int(struct thread *td, vm_offset_t addr, u_int32_t v)
{
struct iovec iov;
struct uio uio;
PROC_LOCK_ASSERT(td->td_proc, MA_NOTOWNED);
iov.iov_base = (caddr_t) &v;
iov.iov_len = sizeof(u_int32_t);
uio.uio_iov = &iov;
@ -1836,6 +1840,8 @@ ptrace_read_register(struct thread *td, int regno)
static int
ptrace_clear_bpt(struct thread *td, struct mdbpt *bpt)
{
PROC_LOCK_ASSERT(td->td_proc, MA_NOTOWNED);
return ptrace_write_int(td, bpt->addr, bpt->contents);
}
@ -1844,6 +1850,8 @@ ptrace_set_bpt(struct thread *td, struct mdbpt *bpt)
{
int error;
u_int32_t bpins = 0x00000080;
PROC_LOCK_ASSERT(td->td_proc, MA_NOTOWNED);
error = ptrace_read_int(td, bpt->addr, &bpt->contents);
if (error)
return error;
@ -1853,12 +1861,20 @@ ptrace_set_bpt(struct thread *td, struct mdbpt *bpt)
int
ptrace_clear_single_step(struct thread *td)
{
struct proc *p;
p = td->td_proc;
PROC_LOCK_ASSERT(p, MA_OWNED);
if (td->td_md.md_flags & MDTD_STEP2) {
PROC_UNLOCK(p);
ptrace_clear_bpt(td, &td->td_md.md_sstep[1]);
ptrace_clear_bpt(td, &td->td_md.md_sstep[0]);
PROC_LOCK(p);
td->td_md.md_flags &= ~MDTD_STEP2;
} else if (td->td_md.md_flags & MDTD_STEP1) {
PROC_UNLOCK(p);
ptrace_clear_bpt(td, &td->td_md.md_sstep[0]);
PROC_LOCK(p);
td->td_md.md_flags &= ~MDTD_STEP1;
}
return 0;
@ -1867,6 +1883,7 @@ ptrace_clear_single_step(struct thread *td)
int
ptrace_single_step(struct thread *td)
{
struct proc *p;
int error;
vm_offset_t pc = td->td_frame->tf_regs[FRAME_PC];
alpha_instruction ins;
@ -1876,9 +1893,11 @@ ptrace_single_step(struct thread *td)
if (td->td_md.md_flags & (MDTD_STEP1|MDTD_STEP2))
panic("ptrace_single_step: step breakpoints not removed");
p = td->td_proc;
PROC_UNLOCK(p);
error = ptrace_read_int(td, pc, &ins.bits);
if (error)
return (error);
goto out;
switch (ins.branch_format.opcode) {
@ -1918,18 +1937,20 @@ ptrace_single_step(struct thread *td)
td->td_md.md_sstep[0].addr = addr[0];
error = ptrace_set_bpt(td, &td->td_md.md_sstep[0]);
if (error)
return (error);
goto out;
if (count == 2) {
td->td_md.md_sstep[1].addr = addr[1];
error = ptrace_set_bpt(td, &td->td_md.md_sstep[1]);
if (error) {
ptrace_clear_bpt(td, &td->td_md.md_sstep[0]);
return (error);
goto out;
}
td->td_md.md_flags |= MDTD_STEP2;
} else
td->td_md.md_flags |= MDTD_STEP1;
out:
PROC_LOCK(p);
return (error);
}

View File

@ -403,8 +403,12 @@ trap(a0, a1, a2, entry, framep)
case ALPHA_IF_CODE_BUGCHK:
if (td->td_md.md_flags & (MDTD_STEP1|MDTD_STEP2)) {
mtx_lock(&Giant);
PROC_LOCK(p);
_PHOLD(p);
ptrace_clear_single_step(td);
td->td_frame->tf_regs[FRAME_PC] -= 4;
_PRELE(p);
PROC_UNLOCK(p);
mtx_unlock(&Giant);
}
ucode = a0; /* trap type */

View File

@ -327,6 +327,8 @@ ptrace_read_int(struct thread *td, vm_offset_t addr, u_int32_t *v)
{
struct iovec iov;
struct uio uio;
PROC_LOCK_ASSERT(td->td_proc, MA_NOTOWNED);
iov.iov_base = (caddr_t) v;
iov.iov_len = sizeof(u_int32_t);
uio.uio_iov = &iov;
@ -344,6 +346,8 @@ ptrace_write_int(struct thread *td, vm_offset_t addr, u_int32_t v)
{
struct iovec iov;
struct uio uio;
PROC_LOCK_ASSERT(td->td_proc, MA_NOTOWNED);
iov.iov_base = (caddr_t) &v;
iov.iov_len = sizeof(u_int32_t);
uio.uio_iov = &iov;
@ -359,28 +363,38 @@ ptrace_write_int(struct thread *td, vm_offset_t addr, u_int32_t v)
int
ptrace_single_step(struct thread *td)
{
struct proc *p;
int error;
KASSERT(td->td_md.md_ptrace_instr == 0,
("Didn't clear single step"));
p = td->td_proc;
PROC_UNLOCK(p);
error = ptrace_read_int(td, td->td_frame->tf_pc + 4,
&td->td_md.md_ptrace_instr);
if (error)
return (error);
goto out;
error = ptrace_write_int(td, td->td_frame->tf_pc + 4,
PTRACE_BREAKPOINT);
if (error)
td->td_md.md_ptrace_instr = 0;
td->td_md.md_ptrace_addr = td->td_frame->tf_pc + 4;
out:
PROC_LOCK(p);
return (error);
}
int
ptrace_clear_single_step(struct thread *td)
{
struct proc *p;
if (td->td_md.md_ptrace_instr) {
p = td->td_proc;
PROC_UNLOCK(p);
ptrace_write_int(td, td->td_md.md_ptrace_addr,
td->td_md.md_ptrace_instr);
PROC_LOCK(p);
td->td_md.md_ptrace_instr = 0;
}
return (0);

View File

@ -261,7 +261,11 @@ undefinedinstruction(trapframe_t *frame)
break;
if (fault_code & FAULT_USER && fault_instruction == PTRACE_BREAKPOINT) {
PROC_LOCK(td->td_proc);
_PHOLD(td->td_proc);
ptrace_clear_single_step(td);
_PRELE(td->td_proc);
PROC_UNLOCK(td->td_proc);
return;
}

View File

@ -99,6 +99,10 @@ pfs_visible(struct thread *td, struct pfs_node *pn, pid_t pid, struct proc **p)
if (pid != NO_PID) {
if ((proc = pfind(pid)) == NULL)
PFS_RETURN (0);
if (proc->p_flag & P_WEXIT) {
PROC_UNLOCK(proc);
PFS_RETURN (0);
}
if (p_cansee(td, proc) != 0 ||
(pn->pn_vis != NULL && !(pn->pn_vis)(td, proc, pn))) {
PROC_UNLOCK(proc);
@ -706,6 +710,10 @@ pfs_readlink(struct vop_readlink_args *va)
if (pvd->pvd_pid != NO_PID) {
if ((proc = pfind(pvd->pvd_pid)) == NULL)
PFS_RETURN (EIO);
if (proc->p_flag & P_WEXIT) {
PROC_UNLOCK(proc);
PFS_RETURN (EIO);
}
_PHOLD(proc);
PROC_UNLOCK(proc);
}

View File

@ -356,6 +356,12 @@ linux_ptrace(struct thread *td, struct linux_ptrace_args *uap)
break;
}
/* Exiting processes can't be debugged. */
if ((p->p_flag & P_WEXIT) != 0) {
error = ESRCH;
goto fail;
}
if ((error = p_candebug(td, p)) != 0)
goto fail;

View File

@ -1097,6 +1097,7 @@ ia64_flush_dirty(struct thread *td, struct _special *r)
r->rnat = (bspst > kstk && (bspst & 0x1ffL) < (kstk & 0x1ffL))
? *(uint64_t*)(kstk | 0x1f8L) : rnat;
} else {
PHOLD(td->td_proc);
iov.iov_base = (void*)(uintptr_t)kstk;
iov.iov_len = r->ndirty;
uio.uio_iov = &iov;
@ -1114,6 +1115,7 @@ ia64_flush_dirty(struct thread *td, struct _special *r)
*/
if (uio.uio_resid != 0 && error == 0)
error = ENOSPC;
PRELE(td->td_proc);
}
r->bspstore += r->ndirty;

View File

@ -175,7 +175,29 @@ exit1(struct thread *td, int rv)
*/
}
/*
* Wakeup anyone in procfs' PIOCWAIT. They should have a hold
* on our vmspace, so we should block below until they have
* released their reference to us. Note that if they have
* requested S_EXIT stops we will block here until they ack
* via PIOCCONT.
*/
_STOPEVENT(p, S_EXIT, rv);
/*
* Note that we are exiting and do another wakeup of anyone in
* PIOCWAIT in case they aren't listening for S_EXIT stops or
* decided to wait again after we told them we are exiting.
*/
p->p_flag |= P_WEXIT;
wakeup(&p->p_stype);
/*
* Wait for any processes that have a hold on our vmspace to
* release their reference.
*/
while (p->p_lock > 0)
msleep(&p->p_lock, &p->p_mtx, PWAIT, "exithold", 0);
PROC_LOCK(p->p_pptr);
sigqueue_take(p->p_ksi);
@ -209,11 +231,6 @@ exit1(struct thread *td, int rv)
mtx_unlock(&ppeers_lock);
}
PROC_LOCK(p);
_STOPEVENT(p, S_EXIT, rv);
wakeup(&p->p_stype); /* Wakeup anyone in procfs' PIOCWAIT */
PROC_UNLOCK(p);
/*
* Check if any loadable modules need anything done at process exit.
* E.g. SYSV IPC stuff

View File

@ -148,7 +148,9 @@ kse_switchin(struct thread *td, struct kse_switchin_args *uap)
td->td_mailbox = uap->tmbx;
td->td_pflags |= TDP_CAN_UNBIND;
}
PROC_LOCK(td->td_proc);
if (td->td_proc->p_flag & P_TRACED) {
_PHOLD(td->td_proc);
if (tmbx.tm_dflags & TMDF_SSTEP)
ptrace_single_step(td);
else
@ -160,7 +162,9 @@ kse_switchin(struct thread *td, struct kse_switchin_args *uap)
ku->ku_flags |= KUF_DOUPCALL;
mtx_unlock_spin(&sched_lock);
}
_PRELE(td->td_proc);
}
PROC_UNLOCK(td->td_proc);
}
return ((error == 0) ? EJUSTRETURN : error);
}
@ -782,8 +786,13 @@ kse_create(struct thread *td, struct kse_create_args *uap)
*/
cpu_set_upcall_kse(newtd, newku->ku_func,
newku->ku_mailbox, &newku->ku_stack);
if (p->p_flag & P_TRACED)
PROC_LOCK(p);
if (p->p_flag & P_TRACED) {
_PHOLD(p);
ptrace_clear_single_step(newtd);
_PRELE(p);
}
PROC_UNLOCK(p);
}
}
@ -1376,8 +1385,13 @@ thread_userret(struct thread *td, struct trapframe *frame)
if (!(ku->ku_mflags & KMF_NOUPCALL)) {
cpu_set_upcall_kse(td, ku->ku_func, ku->ku_mailbox,
&ku->ku_stack);
if (p->p_flag & P_TRACED)
PROC_LOCK(p);
if (p->p_flag & P_TRACED) {
_PHOLD(p);
ptrace_clear_single_step(td);
_PRELE(p);
}
PROC_UNLOCK(p);
error = suword32(&ku->ku_mailbox->km_lwp,
td->td_tid);
if (error)

View File

@ -212,30 +212,24 @@ proc_sstep(struct thread *td)
int
proc_rwmem(struct proc *p, struct uio *uio)
{
struct vmspace *vm;
vm_map_t map;
vm_object_t backing_object, object = NULL;
vm_offset_t pageno = 0; /* page number */
vm_prot_t reqprot;
int error, refcnt, writing;
int error, writing;
/*
* if the vmspace is in the midst of being deallocated or the
* process is exiting, don't try to grab anything. The page table
* usage in that process can be messed up.
* Assert that someone has locked this vmspace. (Should be
* curthread but we can't assert that.) This keeps the process
* from exiting out from under us until this operation completes.
*/
vm = p->p_vmspace;
if ((p->p_flag & P_WEXIT))
return (EFAULT);
do {
if ((refcnt = vm->vm_refcnt) < 1)
return (EFAULT);
} while (!atomic_cmpset_int(&vm->vm_refcnt, refcnt, refcnt + 1));
KASSERT(p->p_lock >= 1, ("%s: process %p (pid %d) not held", __func__,
p, p->p_pid));
/*
* The map we want...
*/
map = &vm->vm_map;
map = &p->p_vmspace->vm_map;
writing = uio->uio_rw == UIO_WRITE;
reqprot = writing ? (VM_PROT_WRITE | VM_PROT_OVERRIDE_WRITE) :
@ -338,7 +332,6 @@ proc_rwmem(struct proc *p, struct uio *uio)
} while (error == 0 && uio->uio_resid > 0);
vmspace_free(vm);
return (error);
}
@ -558,6 +551,11 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
}
}
AUDIT_ARG(process, p);
if ((p->p_flag & P_WEXIT) != 0) {
error = ESRCH;
goto fail;
}
if ((error = p_cansee(td, p)) != 0)
goto fail;
@ -665,11 +663,14 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
break;
}
/* Keep this process around until we finish this request. */
_PHOLD(p);
#ifdef FIX_SSTEP
/*
* Single step fixup ala procfs
*/
FIX_SSTEP(td2); /* XXXKSE */
FIX_SSTEP(td2);
#endif
/*
@ -683,9 +684,7 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
/* set my trace flag and "owner" so it can read/write me */
p->p_flag |= P_TRACED;
p->p_oppid = p->p_pptr->p_pid;
PROC_UNLOCK(p);
sx_xunlock(&proctree_lock);
return (0);
break;
case PT_ATTACH:
/* security check done above */
@ -701,40 +700,24 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
goto sendsig; /* in PT_CONTINUE below */
case PT_CLEARSTEP:
_PHOLD(p);
error = ptrace_clear_single_step(td2);
_PRELE(p);
if (error)
goto fail;
PROC_UNLOCK(p);
return (0);
break;
case PT_SETSTEP:
_PHOLD(p);
error = ptrace_single_step(td2);
_PRELE(p);
if (error)
goto fail;
PROC_UNLOCK(p);
return (0);
break;
case PT_SUSPEND:
_PHOLD(p);
mtx_lock_spin(&sched_lock);
td2->td_flags |= TDF_DBSUSPEND;
mtx_unlock_spin(&sched_lock);
_PRELE(p);
PROC_UNLOCK(p);
return (0);
break;
case PT_RESUME:
_PHOLD(p);
mtx_lock_spin(&sched_lock);
td2->td_flags &= ~TDF_DBSUSPEND;
mtx_unlock_spin(&sched_lock);
_PRELE(p);
PROC_UNLOCK(p);
return (0);
break;
case PT_STEP:
case PT_CONTINUE:
@ -745,20 +728,14 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
/* Zero means do not send any signal */
if (data < 0 || data > _SIG_MAXSIG) {
error = EINVAL;
goto fail;
break;
}
_PHOLD(p);
switch (req) {
case PT_STEP:
PROC_UNLOCK(p);
error = ptrace_single_step(td2);
if (error) {
PRELE(p);
goto fail_noproc;
}
PROC_LOCK(p);
if (error)
goto out;
break;
case PT_TO_SCE:
p->p_stops |= S_PT_SCE;
@ -772,15 +749,10 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
}
if (addr != (void *)1) {
PROC_UNLOCK(p);
error = ptrace_set_pc(td2, (u_long)(uintfptr_t)addr);
if (error) {
PRELE(p);
goto fail_noproc;
}
PROC_LOCK(p);
if (error)
break;
}
_PRELE(p);
if (req == PT_DETACH) {
/* reset process parent */
@ -810,8 +782,10 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
}
sendsig:
if (proctree_locked)
if (proctree_locked) {
sx_xunlock(&proctree_lock);
proctree_locked = 0;
}
/* deliver or queue signal */
mtx_lock_spin(&sched_lock);
td2->td_flags &= ~TDF_XSIG;
@ -842,8 +816,7 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
if (data)
psignal(p, data);
PROC_UNLOCK(p);
return (0);
break;
case PT_WRITE_I:
case PT_WRITE_D:
@ -879,10 +852,10 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
}
if (!write)
td->td_retval[0] = tmp;
return (error);
PROC_LOCK(p);
break;
case PT_IO:
PROC_UNLOCK(p);
#ifdef COMPAT_IA32
if (wrap32) {
piod32 = addr;
@ -918,8 +891,10 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
uio.uio_rw = UIO_WRITE;
break;
default:
return (EINVAL);
error = EINVAL;
goto out;
}
PROC_UNLOCK(p);
error = proc_rwmem(p, &uio);
#ifdef COMPAT_IA32
if (wrap32)
@ -927,59 +902,43 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
else
#endif
piod->piod_len -= uio.uio_resid;
return (error);
PROC_LOCK(p);
break;
case PT_KILL:
data = SIGKILL;
goto sendsig; /* in PT_CONTINUE above */
case PT_SETREGS:
_PHOLD(p);
error = PROC_WRITE(regs, td2, addr);
_PRELE(p);
PROC_UNLOCK(p);
return (error);
break;
case PT_GETREGS:
_PHOLD(p);
error = PROC_READ(regs, td2, addr);
_PRELE(p);
PROC_UNLOCK(p);
return (error);
break;
case PT_SETFPREGS:
_PHOLD(p);
error = PROC_WRITE(fpregs, td2, addr);
_PRELE(p);
PROC_UNLOCK(p);
return (error);
break;
case PT_GETFPREGS:
_PHOLD(p);
error = PROC_READ(fpregs, td2, addr);
_PRELE(p);
PROC_UNLOCK(p);
return (error);
break;
case PT_SETDBREGS:
_PHOLD(p);
error = PROC_WRITE(dbregs, td2, addr);
_PRELE(p);
PROC_UNLOCK(p);
return (error);
break;
case PT_GETDBREGS:
_PHOLD(p);
error = PROC_READ(dbregs, td2, addr);
_PRELE(p);
PROC_UNLOCK(p);
return (error);
break;
case PT_LWPINFO:
if (data == 0 || data > sizeof(*pl))
return (EINVAL);
if (data == 0 || data > sizeof(*pl)) {
error = EINVAL;
break;
}
pl = addr;
_PHOLD(p);
pl->pl_lwpid = td2->td_tid;
if (td2->td_flags & TDF_XSIG)
pl->pl_event = PL_EVENT_SIGNAL;
@ -994,19 +953,16 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
}
pl->pl_sigmask = td2->td_sigmask;
pl->pl_siglist = td2->td_siglist;
_PRELE(p);
PROC_UNLOCK(p);
return (0);
break;
case PT_GETNUMLWPS:
td->td_retval[0] = p->p_numthreads;
PROC_UNLOCK(p);
return (0);
break;
case PT_GETLWPLIST:
if (data <= 0) {
PROC_UNLOCK(p);
return (EINVAL);
error = EINVAL;
break;
}
num = imin(p->p_numthreads, data);
PROC_UNLOCK(p);
@ -1025,27 +981,27 @@ kern_ptrace(struct thread *td, int req, pid_t pid, void *addr, int data)
free(buf, M_TEMP);
if (!error)
td->td_retval[0] = num;
return (error);
PROC_LOCK(p);
break;
default:
#ifdef __HAVE_PTRACE_MACHDEP
if (req >= PT_FIRSTMACH) {
_PHOLD(p);
PROC_UNLOCK(p);
error = cpu_ptrace(td2, req, addr, data);
PRELE(p);
return (error);
}
PROC_LOCK(p);
} else
#endif
/* Unknown request. */
error = EINVAL;
break;
}
/* Unknown request. */
error = EINVAL;
out:
/* Drop our hold on this process now that the request has completed. */
_PRELE(p);
fail:
PROC_UNLOCK(p);
fail_noproc:
if (proctree_locked)
sx_xunlock(&proctree_lock);
return (error);

View File

@ -792,6 +792,7 @@ MALLOC_DECLARE(M_ZOMBIE);
} while (0)
#define _PHOLD(p) do { \
PROC_LOCK_ASSERT((p), MA_OWNED); \
KASSERT(!((p)->p_flag & P_WEXIT), ("PHOLD of exiting process"));\
(p)->p_lock++; \
if (((p)->p_sflag & PS_INMEM) == 0) \
faultin((p)); \
@ -805,6 +806,8 @@ MALLOC_DECLARE(M_ZOMBIE);
#define _PRELE(p) do { \
PROC_LOCK_ASSERT((p), MA_OWNED); \
(--(p)->p_lock); \
if (((p)->p_flag & P_WEXIT) && (p)->p_lock == 0) \
wakeup(&(p)->p_lock); \
} while (0)
/* Check whether a thread is safe to be swapped out. */