diff options
author | jhb <jhb@FreeBSD.org> | 2010-10-21 19:17:40 +0000 |
---|---|---|
committer | jhb <jhb@FreeBSD.org> | 2010-10-21 19:17:40 +0000 |
commit | a41cfdca06b8bd7e2a18e4826efa4b7043be8df0 (patch) | |
tree | 1cee2d1ca3cce75625ff08d21287b1df1ccfd334 /sys/kern | |
parent | 118814a9ac4ce1506dcde746c7e878c1cdbb86b7 (diff) | |
download | FreeBSD-src-a41cfdca06b8bd7e2a18e4826efa4b7043be8df0.zip FreeBSD-src-a41cfdca06b8bd7e2a18e4826efa4b7043be8df0.tar.gz |
- When disabling ktracing on a process, free any pending requests that
may be left. This fixes a memory leak that can occur when tracing is
disabled on a process via disabling tracing of a specific file (or if
an I/O error occurs with the tracefile) if the process's next system
call is exit(). The trace disabling code clears p_traceflag, so exit1()
doesn't do any KTRACE-related cleanup leading to the leak. I chose to
make the free'ing of pending records synchronous rather than patching
exit1().
- Move KTRACE-specific logic out of kern_(exec|exit|fork).c and into
kern_ktrace.c instead. Make ktrace_mtx private to kern_ktrace.c as a
result.
MFC after: 1 month
Diffstat (limited to 'sys/kern')
-rw-r--r-- | sys/kern/kern_exec.c | 12 | ||||
-rw-r--r-- | sys/kern/kern_exit.c | 32 | ||||
-rw-r--r-- | sys/kern/kern_fork.c | 16 | ||||
-rw-r--r-- | sys/kern/kern_ktrace.c | 126 |
4 files changed, 106 insertions, 80 deletions
diff --git a/sys/kern/kern_exec.c b/sys/kern/kern_exec.c index 3c05530..1e4d690 100644 --- a/sys/kern/kern_exec.c +++ b/sys/kern/kern_exec.c @@ -655,16 +655,8 @@ interpret: setsugid(p); #ifdef KTRACE - if (p->p_tracevp != NULL && - priv_check_cred(oldcred, PRIV_DEBUG_DIFFCRED, 0)) { - mtx_lock(&ktrace_mtx); - p->p_traceflag = 0; - tracevp = p->p_tracevp; - p->p_tracevp = NULL; - tracecred = p->p_tracecred; - p->p_tracecred = NULL; - mtx_unlock(&ktrace_mtx); - } + if (priv_check_cred(oldcred, PRIV_DEBUG_DIFFCRED, 0)) + ktrprocexec(p, &tracecred, &tracevp); #endif /* * Close any file descriptors 0..2 that reference procfs, diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c index 029f1c3..31389e1 100644 --- a/sys/kern/kern_exit.c +++ b/sys/kern/kern_exit.c @@ -121,10 +121,6 @@ exit1(struct thread *td, int rv) struct proc *p, *nq, *q; struct vnode *vtmp; struct vnode *ttyvp = NULL; -#ifdef KTRACE - struct vnode *tracevp; - struct ucred *tracecred; -#endif struct plimit *plim; int locked; @@ -356,33 +352,7 @@ exit1(struct thread *td, int rv) if (ttyvp != NULL) vrele(ttyvp); #ifdef KTRACE - /* - * Disable tracing, then drain any pending records and release - * the trace file. - */ - if (p->p_traceflag != 0) { - PROC_LOCK(p); - mtx_lock(&ktrace_mtx); - p->p_traceflag = 0; - mtx_unlock(&ktrace_mtx); - PROC_UNLOCK(p); - ktrprocexit(td); - PROC_LOCK(p); - mtx_lock(&ktrace_mtx); - tracevp = p->p_tracevp; - p->p_tracevp = NULL; - tracecred = p->p_tracecred; - p->p_tracecred = NULL; - mtx_unlock(&ktrace_mtx); - PROC_UNLOCK(p); - if (tracevp != NULL) { - locked = VFS_LOCK_GIANT(tracevp->v_mount); - vrele(tracevp); - VFS_UNLOCK_GIANT(locked); - } - if (tracecred != NULL) - crfree(tracecred); - } + ktrprocexit(td); #endif /* * Release reference to text vnode diff --git a/sys/kern/kern_fork.c b/sys/kern/kern_fork.c index da2c415..126c668 100644 --- a/sys/kern/kern_fork.c +++ b/sys/kern/kern_fork.c @@ -645,21 +645,7 @@ again: callout_init(&p2->p_itcallout, CALLOUT_MPSAFE); #ifdef KTRACE - /* - * Copy traceflag and tracefile if enabled. - */ - mtx_lock(&ktrace_mtx); - KASSERT(p2->p_tracevp == NULL, ("new process has a ktrace vnode")); - if (p1->p_traceflag & KTRFAC_INHERIT) { - p2->p_traceflag = p1->p_traceflag; - if ((p2->p_tracevp = p1->p_tracevp) != NULL) { - VREF(p2->p_tracevp); - KASSERT(p1->p_tracecred != NULL, - ("ktrace vnode with no cred")); - p2->p_tracecred = crhold(p1->p_tracecred); - } - } - mtx_unlock(&ktrace_mtx); + ktrprocfork(p1, p2); #endif /* diff --git a/sys/kern/kern_ktrace.c b/sys/kern/kern_ktrace.c index bf530e1..6e2285b 100644 --- a/sys/kern/kern_ktrace.c +++ b/sys/kern/kern_ktrace.c @@ -126,7 +126,7 @@ SYSCTL_UINT(_kern_ktrace, OID_AUTO, genio_size, CTLFLAG_RW, &ktr_geniosize, 0, "Maximum size of genio event payload"); static int print_message = 1; -struct mtx ktrace_mtx; +static struct mtx ktrace_mtx; static struct sx ktrace_sx; static void ktrace_init(void *dummy); @@ -134,7 +134,10 @@ static int sysctl_kern_ktrace_request_pool(SYSCTL_HANDLER_ARGS); static u_int ktrace_resize_pool(u_int newsize); static struct ktr_request *ktr_getrequest(int type); static void ktr_submitrequest(struct thread *td, struct ktr_request *req); +static void ktr_freeproc(struct proc *p, struct ucred **uc, + struct vnode **vp); static void ktr_freerequest(struct ktr_request *req); +static void ktr_freerequest_locked(struct ktr_request *req); static void ktr_writerequest(struct thread *td, struct ktr_request *req); static int ktrcanset(struct thread *,struct proc *); static int ktrsetchildren(struct thread *,struct proc *,int,int,struct vnode *); @@ -375,11 +378,43 @@ static void ktr_freerequest(struct ktr_request *req) { + mtx_lock(&ktrace_mtx); + ktr_freerequest_locked(req); + mtx_unlock(&ktrace_mtx); +} + +static void +ktr_freerequest_locked(struct ktr_request *req) +{ + + mtx_assert(&ktrace_mtx, MA_OWNED); if (req->ktr_buffer != NULL) free(req->ktr_buffer, M_KTRACE); - mtx_lock(&ktrace_mtx); STAILQ_INSERT_HEAD(&ktr_free, req, ktr_list); - mtx_unlock(&ktrace_mtx); +} + +/* + * Disable tracing for a process and release all associated resources. + * The caller is responsible for releasing a reference on the returned + * vnode and credentials. + */ +static void +ktr_freeproc(struct proc *p, struct ucred **uc, struct vnode **vp) +{ + struct ktr_request *req; + + PROC_LOCK_ASSERT(p, MA_OWNED); + mtx_assert(&ktrace_mtx, MA_OWNED); + *uc = p->p_tracecred; + p->p_tracecred = NULL; + if (vp != NULL) + *vp = p->p_tracevp; + p->p_tracevp = NULL; + p->p_traceflag = 0; + while ((req = STAILQ_FIRST(&p->p_ktr)) != NULL) { + STAILQ_REMOVE_HEAD(&p->p_ktr, ktr_list); + ktr_freerequest_locked(req); + } } void @@ -432,20 +467,79 @@ ktrsysret(code, error, retval) } /* - * When a process exits, drain per-process asynchronous trace records. + * When a setuid process execs, disable tracing. + * + * XXX: We toss any pending asynchronous records. + */ +void +ktrprocexec(struct proc *p, struct ucred **uc, struct vnode **vp) +{ + + PROC_LOCK_ASSERT(p, MA_OWNED); + mtx_lock(&ktrace_mtx); + ktr_freeproc(p, uc, vp); + mtx_unlock(&ktrace_mtx); +} + +/* + * When a process exits, drain per-process asynchronous trace records + * and disable tracing. */ void ktrprocexit(struct thread *td) { + struct proc *p; + struct ucred *cred; + struct vnode *vp; + int vfslocked; + + p = td->td_proc; + if (p->p_traceflag == 0) + return; ktrace_enter(td); sx_xlock(&ktrace_sx); ktr_drain(td); sx_xunlock(&ktrace_sx); + PROC_LOCK(p); + mtx_lock(&ktrace_mtx); + ktr_freeproc(p, &cred, &vp); + mtx_unlock(&ktrace_mtx); + PROC_UNLOCK(p); + if (vp != NULL) { + vfslocked = VFS_LOCK_GIANT(vp->v_mount); + vrele(vp); + VFS_UNLOCK_GIANT(vfslocked); + } + if (cred != NULL) + crfree(cred); ktrace_exit(td); } /* + * When a process forks, enable tracing in the new process if needed. + */ +void +ktrprocfork(struct proc *p1, struct proc *p2) +{ + + PROC_LOCK_ASSERT(p1, MA_OWNED); + PROC_LOCK_ASSERT(p2, MA_OWNED); + mtx_lock(&ktrace_mtx); + KASSERT(p2->p_tracevp == NULL, ("new process has a ktrace vnode")); + if (p1->p_traceflag & KTRFAC_INHERIT) { + p2->p_traceflag = p1->p_traceflag; + if ((p2->p_tracevp = p1->p_tracevp) != NULL) { + VREF(p2->p_tracevp); + KASSERT(p1->p_tracecred != NULL, + ("ktrace vnode with no cred")); + p2->p_tracecred = crhold(p1->p_tracecred); + } + } + mtx_unlock(&ktrace_mtx); +} + +/* * When a thread returns, drain any asynchronous records generated by the * system call. */ @@ -694,10 +788,7 @@ ktrace(td, uap) if (p->p_tracevp == vp) { if (ktrcanset(td, p)) { mtx_lock(&ktrace_mtx); - cred = p->p_tracecred; - p->p_tracecred = NULL; - p->p_tracevp = NULL; - p->p_traceflag = 0; + ktr_freeproc(p, &cred, NULL); mtx_unlock(&ktrace_mtx); vrele_count++; crfree(cred); @@ -864,14 +955,9 @@ ktrops(td, p, ops, facs, vp) p->p_traceflag |= KTRFAC_ROOT; } else { /* KTROP_CLEAR */ - if (((p->p_traceflag &= ~facs) & KTRFAC_MASK) == 0) { + if (((p->p_traceflag &= ~facs) & KTRFAC_MASK) == 0) /* no more tracing */ - p->p_traceflag = 0; - tracevp = p->p_tracevp; - p->p_tracevp = NULL; - tracecred = p->p_tracecred; - p->p_tracecred = NULL; - } + ktr_freeproc(p, &tracecred, &tracevp); } mtx_unlock(&ktrace_mtx); PROC_UNLOCK(p); @@ -1036,10 +1122,7 @@ ktr_writerequest(struct thread *td, struct ktr_request *req) PROC_LOCK(p); if (p->p_tracevp == vp) { mtx_lock(&ktrace_mtx); - p->p_tracevp = NULL; - p->p_traceflag = 0; - cred = p->p_tracecred; - p->p_tracecred = NULL; + ktr_freeproc(p, &cred, NULL); mtx_unlock(&ktrace_mtx); vrele_count++; } @@ -1051,11 +1134,6 @@ ktr_writerequest(struct thread *td, struct ktr_request *req) } sx_sunlock(&allproc_lock); - /* - * We can't clear any pending requests in threads that have cached - * them but not yet committed them, as those are per-thread. The - * thread will have to clear it itself on system call return. - */ vfslocked = VFS_LOCK_GIANT(vp->v_mount); while (vrele_count-- > 0) vrele(vp); |