Author: jhb
Date: Thu Oct 21 19:17:40 2010
New Revision: 214158
URL: http://svn.freebsd.org/changeset/base/214158
Log:
- 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
Modified:
head/sys/kern/kern_exec.c
head/sys/kern/kern_exit.c
head/sys/kern/kern_fork.c
head/sys/kern/kern_ktrace.c
head/sys/sys/ktrace.h
Modified: head/sys/kern/kern_exec.c
==============================================================================
--- head/sys/kern/kern_exec.c Thu Oct 21 19:11:14 2010 (r214157)
+++ head/sys/kern/kern_exec.c Thu Oct 21 19:17:40 2010 (r214158)
@@ -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,
Modified: head/sys/kern/kern_exit.c
==============================================================================
--- head/sys/kern/kern_exit.c Thu Oct 21 19:11:14 2010 (r214157)
+++ head/sys/kern/kern_exit.c Thu Oct 21 19:17:40 2010 (r214158)
@@ -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
Modified: head/sys/kern/kern_fork.c
==============================================================================
--- head/sys/kern/kern_fork.c Thu Oct 21 19:11:14 2010 (r214157)
+++ head/sys/kern/kern_fork.c Thu Oct 21 19:17:40 2010 (r214158)
@@ -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
/*
Modified: head/sys/kern/kern_ktrace.c
==============================================================================
--- head/sys/kern/kern_ktrace.c Thu Oct 21 19:11:14 2010 (r214157)
+++ head/sys/kern/kern_ktrace.c Thu Oct 21 19:17:40 2010 (r214158)
@@ -126,7 +126,7 @@ SYSCTL_UINT(_kern_ktrace, OID_AUTO, geni
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_po
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, stru
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, stru
}
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);
Modified: head/sys/sys/ktrace.h
==============================================================================
--- head/sys/sys/ktrace.h Thu Oct 21 19:11:14 2010 (r214157)
+++ head/sys/sys/ktrace.h Thu Oct 21 19:17:40 2010 (r214158)
@@ -191,8 +191,6 @@ struct stat;
#define KTRFAC_DROP 0x20000000 /* last event was dropped */
#ifdef _KERNEL
-extern struct mtx ktrace_mtx;
-
void ktrnamei(char *);
void ktrcsw(int, int);
void ktrpsig(int, sig_t, sigset_t *, int);
@@ -200,7 +198,9 @@ void ktrgenio(int, enum uio_rw, struct u
void ktrsyscall(int, int narg, register_t args[]);
void ktrsysctl(int *name, u_int namelen);
void ktrsysret(int, int, register_t);
+void ktrprocexec(struct proc *, struct ucred **, struct vnode **);
void ktrprocexit(struct thread *);
+void ktrprocfork(struct proc *, struct proc *);
void ktruserret(struct thread *);
void ktrstruct(const char *, void *, size_t);
#define ktrsockaddr(s) \
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
