Index: share/man/man4/filemon.4 =================================================================== --- share/man/man4/filemon.4 +++ share/man/man4/filemon.4 @@ -31,7 +31,7 @@ .\" .\" $FreeBSD$ .\" -.Dd January 28, 2016 +.Dd March 7, 2016 .Dt FILEMON 4 .Os .Sh NAME @@ -186,14 +186,5 @@ device appeared in .Fx 9.1 . .Sh BUGS -Loading -.Nm -may reduce system performance for the noted syscalls. -.Pp -Only children of the set process are logged. -Processes can escape being traced by double forking. -This is not seen as a problem as the intended use is build monitoring, which -does not make sense to have daemons for. -.Pp Unloading the module may panic the system, thus requires using .Ic kldunload -f . Index: sys/dev/filemon/filemon.c =================================================================== --- sys/dev/filemon/filemon.c +++ sys/dev/filemon/filemon.c @@ -46,7 +46,6 @@ #include #include #include -#include #include #include #include @@ -80,23 +79,92 @@ MALLOC_DECLARE(M_FILEMON); MALLOC_DEFINE(M_FILEMON, "filemon", "File access monitor"); +/* + * The filemon->lock protects several things currently: + * - fname1/fname2/msgbufr are pre-allocated and used per syscall + * for logging and copyins rather than stack variables. + * - Serializing the filemon's log output. + * - Preventing inheritance or removal of the filemon into proc.p_filemon. + */ struct filemon { - TAILQ_ENTRY(filemon) link; /* Link into the in-use list. */ - struct sx lock; /* Lock mutex for this filemon. */ + struct sx lock; /* Lock for this filemon. */ struct file *fp; /* Output file pointer. */ - struct proc *p; /* The process being monitored. */ char fname1[MAXPATHLEN]; /* Temporary filename buffer. */ char fname2[MAXPATHLEN]; /* Temporary filename buffer. */ char msgbufr[1024]; /* Output message buffer. */ + volatile u_int refcnt; /* Pointer reference count. */ }; -static TAILQ_HEAD(, filemon) filemons_inuse = TAILQ_HEAD_INITIALIZER(filemons_inuse); -static TAILQ_HEAD(, filemon) filemons_free = TAILQ_HEAD_INITIALIZER(filemons_free); -static struct sx access_lock; - static struct cdev *filemon_dev; -#include "filemon_lock.c" +static __inline struct filemon * +filemon_acquire(struct filemon *filemon) +{ + + if (filemon != NULL) + refcount_acquire(&filemon->refcnt); + return (filemon); +} + +static __inline void +filemon_release(struct filemon *filemon) +{ + + refcount_release(&filemon->refcnt); +} + +static void +filemon_drain(struct filemon *filemon) +{ + + while (filemon->refcnt > 0) + pause("filemon", hz / 10); +} + +/* + * Acquire the proc's p_filemon reference and lock the filemon. + * The proc's p_filemon may not match this filemon on return. + */ +static struct filemon * +filemon_proc_get(struct proc *p) +{ + struct filemon *filemon; + + PROC_LOCK(p); + filemon = filemon_acquire(p->p_filemon); + PROC_UNLOCK(p); + + if (filemon == NULL) + return (NULL); + /* + * The p->p_filemon may have changed by now. That case is handled + * by the exit and fork hooks and filemon_attach_proc specially. + */ + sx_xlock(&filemon->lock); + return (filemon); +} + +static void +filemon_proc_drop(struct proc *p) +{ + + KASSERT(p->p_filemon != NULL, ("%s: proc %p NULL p_filemon", + __func__, p)); + sx_assert(&p->p_filemon->lock, SA_XLOCKED); + PROC_LOCK(p); + filemon_release(p->p_filemon); + p->p_filemon = NULL; + PROC_UNLOCK(p); +} + +static __inline void +filemon_drop(struct filemon *filemon) +{ + + sx_xunlock(&filemon->lock); + filemon_release(filemon); +} + #include "filemon_wrapper.c" static void @@ -115,35 +183,154 @@ filemon_output(filemon, filemon->msgbufr, len); } +/* + * Write the footer and destroy the filemon. Only called from the devfs + * file handle destructor or on error creating the filemon. + */ static void -filemon_dtr(void *data) +filemon_free(struct filemon *filemon) { - struct filemon *filemon = data; + size_t len; + struct timeval now; + + KASSERT(filemon->refcnt == 0, ("%p: filemon %p refcount non-zero", + __func__, filemon)); + + if (filemon->fp != NULL) { + getmicrotime(&now); + + len = snprintf(filemon->msgbufr, + sizeof(filemon->msgbufr), + "# Stop %ju.%06ju\n# Bye bye\n", + (uintmax_t)now.tv_sec, (uintmax_t)now.tv_usec); + + filemon_output(filemon, filemon->msgbufr, len); + fdrop(filemon->fp, curthread); + } + + sx_destroy(&filemon->lock); + free(filemon, M_FILEMON); +} + +/* + * Invalidate the passed filemon in all processes. + */ +static void +filemon_untrack_processes(struct filemon *filemon) +{ + struct proc *p; - if (filemon != NULL) { - struct file *fp; + sx_assert(&filemon->lock, SA_XLOCKED); - /* Follow same locking order as filemon_pid_check. */ - filemon_lock_write(); - sx_xlock(&filemon->lock); + /* First reference is in the cdevpriv. */ + if (filemon->refcnt == 1) + return; - /* Remove from the in-use list. */ - TAILQ_REMOVE(&filemons_inuse, filemon, link); + /* + * Processes in this list won't go away while here since + * filemon_event_process_exit() will lock on filemon->lock + * which we hold. + */ + sx_slock(&allproc_lock); + FOREACH_PROC_IN_SYSTEM(p) { + /* + * No PROC_LOCK is needed to compare here since it is + * guaranteed to not change since we have its filemon + * locked. Everything that changes this p_filemon will + * be locked on it. + */ + if (p->p_filemon == filemon) + filemon_proc_drop(p); + } + sx_sunlock(&allproc_lock); - fp = filemon->fp; - filemon->fp = NULL; - filemon->p = NULL; + /* + * It's possible some references were acquired but will be + * dropped shortly as they are restricted from being + * inherited. + */ +} - /* Add to the free list. */ - TAILQ_INSERT_TAIL(&filemons_free, filemon, link); +/* The devfs file is being closed. Untrace all processes and cleanup. */ +static void +filemon_dtr(void *data) +{ + struct filemon *filemon = data; - /* Give up write access. */ - sx_xunlock(&filemon->lock); - filemon_unlock_write(); + if (filemon == NULL) + return; - if (fp != NULL) - fdrop(fp, curthread); + sx_xlock(&filemon->lock); + filemon_untrack_processes(filemon); + filemon_drop(filemon); + /* + * Some references may remain after untracking, but they won't + * be inherited. Wait for them. + */ + filemon_drain(filemon); + filemon_free(filemon); +} + +/* Attach the filemon to the process. */ +static int +filemon_attach_proc(struct filemon *filemon, struct proc *p) +{ + struct filemon *filemon2; + + sx_assert(&filemon->lock, SA_XLOCKED); + PROC_LOCK_ASSERT(p, MA_OWNED); + KASSERT((p->p_flag & P_WEXIT) == 0, + ("%s: filemon %p attaching to exiting process %p", + __func__, filemon, p)); + + if (p->p_filemon == filemon) + return (0); + + /* + * Historic behavior of filemon has been to let a child initiate + * tracing on itself and cease tracing in the parent. Bmake + * .META + .MAKE relies on this. It is only relevant for attaching to + * curproc. + */ + if (p->p_filemon != NULL) { + /* + * Don't allow truncating other process traces. It is + * not really intended to trace procs other than curproc + * anyhow. + */ + if (p != curproc) + return (EBUSY); + + /* + * Keep the target filemon around while unlocked to avoid + * racing with close() on the devfs handle. + */ + (void)filemon_acquire(filemon); + do { + PROC_UNLOCK(p); + sx_xunlock(&filemon->lock); + while ((filemon2 = filemon_proc_get(p)) != NULL) { + /* It may have changed. */ + if (p->p_filemon == filemon2) + filemon_proc_drop(p); + filemon_drop(filemon2); + } + sx_xlock(&filemon->lock); + PROC_LOCK(p); + /* + * It may have been attached to, though unlikely. + * Try again if needed. + */ + } while (p->p_filemon != NULL); + filemon_release(filemon); + KASSERT(p->p_filemon == NULL, + ("%s: proc %p didn't detach filemon %p", __func__, p, + p->p_filemon)); } + + p->p_filemon = filemon_acquire(filemon); + + return (0); } static int @@ -176,10 +363,16 @@ /* Set the monitored process ID. */ case FILEMON_SET_PID: + /* Invalidate any existing processes already set. */ + filemon_untrack_processes(filemon); + error = pget(*((pid_t *)data), PGET_CANDEBUG | PGET_NOTWEXIT, &p); if (error == 0) { - filemon->p = p; + KASSERT(p->p_filemon != filemon, + ("%s: proc %p didn't untrack filemon %p", + __func__, p, filemon)); + error = filemon_attach_proc(filemon, p); PROC_UNLOCK(p); } break; @@ -197,35 +390,21 @@ filemon_open(struct cdev *dev, int oflags __unused, int devtype __unused, struct thread *td __unused) { + int error; struct filemon *filemon; - /* Get exclusive write access. */ - filemon_lock_write(); + filemon = malloc(sizeof(*filemon), M_FILEMON, + M_WAITOK | M_ZERO); + sx_init(&filemon->lock, "filemon"); + refcount_init(&filemon->refcnt, 1); - if ((filemon = TAILQ_FIRST(&filemons_free)) != NULL) - TAILQ_REMOVE(&filemons_free, filemon, link); - - /* Give up write access. */ - filemon_unlock_write(); - - if (filemon == NULL) { - filemon = malloc(sizeof(struct filemon), M_FILEMON, - M_WAITOK | M_ZERO); - sx_init(&filemon->lock, "filemon"); + error = devfs_set_cdevpriv(filemon, filemon_dtr); + if (error != 0) { + refcount_release(&filemon->refcnt); + filemon_free(filemon); } - devfs_set_cdevpriv(filemon, filemon_dtr); - - /* Get exclusive write access. */ - filemon_lock_write(); - - /* Add to the in-use list. */ - TAILQ_INSERT_TAIL(&filemons_inuse, filemon, link); - - /* Give up write access. */ - filemon_unlock_write(); - - return (0); + return (error); } static int @@ -239,7 +418,6 @@ static void filemon_load(void *dummy __unused) { - sx_init(&access_lock, "filemons_inuse"); /* Install the syscall wrappers. */ filemon_wrapper_install(); @@ -251,38 +429,11 @@ static int filemon_unload(void) { - struct filemon *filemon; - int error = 0; - /* Get exclusive write access. */ - filemon_lock_write(); + destroy_dev(filemon_dev); + filemon_wrapper_deinstall(); - if (TAILQ_FIRST(&filemons_inuse) != NULL) - error = EBUSY; - else { - destroy_dev(filemon_dev); - - /* Deinstall the syscall wrappers. */ - filemon_wrapper_deinstall(); - } - - /* Give up write access. */ - filemon_unlock_write(); - - if (error == 0) { - /* free() filemon structs free list. */ - filemon_lock_write(); - while ((filemon = TAILQ_FIRST(&filemons_free)) != NULL) { - TAILQ_REMOVE(&filemons_free, filemon, link); - sx_destroy(&filemon->lock); - free(filemon, M_FILEMON); - } - filemon_unlock_write(); - - sx_destroy(&access_lock); - } - - return (error); + return (0); } static int Index: sys/dev/filemon/filemon_lock.c =================================================================== --- sys/dev/filemon/filemon_lock.c +++ /dev/null @@ -1,57 +0,0 @@ -/*- - * Copyright (c) 2009-2011, Juniper Networks, Inc. - * Copyright (c) 2015, EMC Corp. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY JUNIPER NETWORKS AND CONTRIBUTORS ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL JUNIPER NETWORKS OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - */ - -#include -__FBSDID("$FreeBSD$"); - -static __inline void -filemon_lock_read(void) -{ - - sx_slock(&access_lock); -} - -static __inline void -filemon_unlock_read(void) -{ - - sx_sunlock(&access_lock); -} - -static __inline void -filemon_lock_write(void) -{ - - sx_xlock(&access_lock); -} - -static __inline void -filemon_unlock_write(void) -{ - - sx_xunlock(&access_lock); -} Index: sys/dev/filemon/filemon_wrapper.c =================================================================== --- sys/dev/filemon/filemon_wrapper.c +++ sys/dev/filemon/filemon_wrapper.c @@ -65,33 +65,6 @@ fo_write(filemon->fp, &auio, curthread->td_ucred, 0, curthread); } -static struct filemon * -filemon_pid_check(struct proc *p) -{ - struct filemon *filemon; - - filemon_lock_read(); - if (TAILQ_EMPTY(&filemons_inuse)) { - filemon_unlock_read(); - return (NULL); - } - sx_slock(&proctree_lock); - while (p->p_pid != 0) { - TAILQ_FOREACH(filemon, &filemons_inuse, link) { - if (p == filemon->p) { - sx_sunlock(&proctree_lock); - sx_xlock(&filemon->lock); - filemon_unlock_read(); - return (filemon); - } - } - p = proc_realparent(p); - } - sx_sunlock(&proctree_lock); - filemon_unlock_read(); - return (NULL); -} - static int filemon_wrapper_chdir(struct thread *td, struct chdir_args *uap) { @@ -101,7 +74,7 @@ struct filemon *filemon; if ((ret = sys_chdir(td, uap)) == 0) { - if ((filemon = filemon_pid_check(curproc)) != NULL) { + if ((filemon = filemon_proc_get(curproc)) != NULL) { copyinstr(uap->path, filemon->fname1, sizeof(filemon->fname1), &done); @@ -111,7 +84,7 @@ filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -126,7 +99,7 @@ char *fullpath, *freepath; size_t len; - if ((filemon = filemon_pid_check(p)) != NULL) { + if ((filemon = filemon_proc_get(p)) != NULL) { fullpath = ""; freepath = NULL; @@ -139,7 +112,7 @@ filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); free(freepath, M_TEMP); } @@ -154,7 +127,7 @@ struct filemon *filemon; if ((ret = sys_open(td, uap)) == 0) { - if ((filemon = filemon_pid_check(curproc)) != NULL) { + if ((filemon = filemon_proc_get(curproc)) != NULL) { copyinstr(uap->path, filemon->fname1, sizeof(filemon->fname1), &done); @@ -177,7 +150,7 @@ curproc->p_pid, filemon->fname1); filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -193,7 +166,7 @@ struct filemon *filemon; if ((ret = sys_openat(td, uap)) == 0) { - if ((filemon = filemon_pid_check(curproc)) != NULL) { + if ((filemon = filemon_proc_get(curproc)) != NULL) { copyinstr(uap->path, filemon->fname1, sizeof(filemon->fname1), &done); @@ -229,7 +202,7 @@ curproc->p_pid, filemon->fname2, filemon->fname1); filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -245,7 +218,7 @@ struct filemon *filemon; if ((ret = sys_rename(td, uap)) == 0) { - if ((filemon = filemon_pid_check(curproc)) != NULL) { + if ((filemon = filemon_proc_get(curproc)) != NULL) { copyinstr(uap->from, filemon->fname1, sizeof(filemon->fname1), &done); copyinstr(uap->to, filemon->fname2, @@ -257,7 +230,7 @@ filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -273,7 +246,7 @@ struct filemon *filemon; if ((ret = sys_link(td, uap)) == 0) { - if ((filemon = filemon_pid_check(curproc)) != NULL) { + if ((filemon = filemon_proc_get(curproc)) != NULL) { copyinstr(uap->path, filemon->fname1, sizeof(filemon->fname1), &done); copyinstr(uap->link, filemon->fname2, @@ -285,7 +258,7 @@ filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -301,7 +274,7 @@ struct filemon *filemon; if ((ret = sys_symlink(td, uap)) == 0) { - if ((filemon = filemon_pid_check(curproc)) != NULL) { + if ((filemon = filemon_proc_get(curproc)) != NULL) { copyinstr(uap->path, filemon->fname1, sizeof(filemon->fname1), &done); copyinstr(uap->link, filemon->fname2, @@ -313,7 +286,7 @@ filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -329,7 +302,7 @@ struct filemon *filemon; if ((ret = sys_linkat(td, uap)) == 0) { - if ((filemon = filemon_pid_check(curproc)) != NULL) { + if ((filemon = filemon_proc_get(curproc)) != NULL) { copyinstr(uap->path1, filemon->fname1, sizeof(filemon->fname1), &done); copyinstr(uap->path2, filemon->fname2, @@ -341,7 +314,7 @@ filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -357,7 +330,7 @@ struct filemon *filemon; if ((ret = sys_stat(td, uap)) == 0) { - if ((filemon = filemon_pid_check(curproc)) != NULL) { + if ((filemon = filemon_proc_get(curproc)) != NULL) { copyinstr(uap->path, filemon->fname1, sizeof(filemon->fname1), &done); @@ -367,7 +340,7 @@ filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -385,7 +358,7 @@ struct filemon *filemon; if ((ret = freebsd32_stat(td, uap)) == 0) { - if ((filemon = filemon_pid_check(curproc)) != NULL) { + if ((filemon = filemon_proc_get(curproc)) != NULL) { copyinstr(uap->path, filemon->fname1, sizeof(filemon->fname1), &done); @@ -395,7 +368,7 @@ filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -408,29 +381,22 @@ { size_t len; struct filemon *filemon; - struct timeval now; - - /* Get timestamp before locking. */ - getmicrotime(&now); - if ((filemon = filemon_pid_check(p)) != NULL) { + if ((filemon = filemon_proc_get(p)) != NULL) { len = snprintf(filemon->msgbufr, sizeof(filemon->msgbufr), "X %d %d %d\n", p->p_pid, p->p_xexit, p->p_xsig); filemon_output(filemon, filemon->msgbufr, len); - /* Check if the monitored process is about to exit. */ - if (filemon->p == p) { - len = snprintf(filemon->msgbufr, - sizeof(filemon->msgbufr), - "# Stop %ju.%06ju\n# Bye bye\n", - (uintmax_t)now.tv_sec, (uintmax_t)now.tv_usec); - - filemon_output(filemon, filemon->msgbufr, len); - filemon->p = NULL; - } + /* + * filemon_untrack_processes() may have dropped this p_filemon + * already while in filemon_proc_get() before acquiring the + * filemon lock. + */ + if (p->p_filemon == filemon) + filemon_proc_drop(p); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -443,7 +409,7 @@ struct filemon *filemon; if ((ret = sys_unlink(td, uap)) == 0) { - if ((filemon = filemon_pid_check(curproc)) != NULL) { + if ((filemon = filemon_proc_get(curproc)) != NULL) { copyinstr(uap->path, filemon->fname1, sizeof(filemon->fname1), &done); @@ -453,7 +419,7 @@ filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -467,14 +433,32 @@ size_t len; struct filemon *filemon; - if ((filemon = filemon_pid_check(p1)) != NULL) { + if ((filemon = filemon_proc_get(p1)) != NULL) { len = snprintf(filemon->msgbufr, sizeof(filemon->msgbufr), "F %d %d\n", p1->p_pid, p2->p_pid); filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + /* + * filemon_untrack_processes() or + * filemon_ioctl(FILEMON_SET_PID) may have changed the parent's + * p_filemon while in filemon_proc_get() before acquiring the + * filemon lock. Only inherit if the parent is still traced by + * this filemon. + */ + if (p1->p_filemon == filemon) { + PROC_LOCK(p2); + /* + * It may have been attached to already by a new + * filemon. + */ + if (p2->p_filemon == NULL) + p2->p_filemon = filemon_acquire(filemon); + PROC_UNLOCK(p2); + } + + filemon_drop(filemon); } } Index: sys/sys/proc.h =================================================================== --- sys/sys/proc.h +++ sys/sys/proc.h @@ -162,6 +162,7 @@ */ struct cpuset; struct filecaps; +struct filemon; struct kaioinfo; struct kaudit_record; struct kdtrace_proc; @@ -580,6 +581,7 @@ struct procdesc *p_procdesc; /* (e) Process descriptor, if any. */ u_int p_treeflag; /* (e) P_TREE flags */ int p_pendingexits; /* (c) Count of pending thread exits. */ + struct filemon *p_filemon; /* (c) filemon-specific data. */ /* End area that is zeroed on creation. */ #define p_endzero p_magic