diff options
author | jhb <jhb@FreeBSD.org> | 2006-02-22 18:57:50 +0000 |
---|---|---|
committer | jhb <jhb@FreeBSD.org> | 2006-02-22 18:57:50 +0000 |
commit | ff9c76bccdce7ca4977a4aea65187a8023b391c2 (patch) | |
tree | 51c4862ad08a234b42fff5687f7a934be133894b /sys/alpha | |
parent | 4793b3db380bcaac1d396101ed11d3b3001642e5 (diff) | |
download | FreeBSD-src-ff9c76bccdce7ca4977a4aea65187a8023b391c2.zip FreeBSD-src-ff9c76bccdce7ca4977a4aea65187a8023b391c2.tar.gz |
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
Diffstat (limited to 'sys/alpha')
-rw-r--r-- | sys/alpha/alpha/machdep.c | 27 | ||||
-rw-r--r-- | sys/alpha/alpha/trap.c | 4 |
2 files changed, 28 insertions, 3 deletions
diff --git a/sys/alpha/alpha/machdep.c b/sys/alpha/alpha/machdep.c index 0a279bb..519126b 100644 --- a/sys/alpha/alpha/machdep.c +++ b/sys/alpha/alpha/machdep.c @@ -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); } diff --git a/sys/alpha/alpha/trap.c b/sys/alpha/alpha/trap.c index a3a47cd..33b4826 100644 --- a/sys/alpha/alpha/trap.c +++ b/sys/alpha/alpha/trap.c @@ -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 */ |