Date: Mon, 21 Mar 2016 20:29:28 +0000 (UTC) From: Bryan Drewery <bdrewery@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r297156 - in head: share/man/man4 sys/dev/filemon sys/sys Message-ID: <201603212029.u2LKTSJp010970@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: bdrewery Date: Mon Mar 21 20:29:27 2016 New Revision: 297156 URL: https://svnweb.freebsd.org/changeset/base/297156 Log: Track filemon usage via a proc.p_filemon pointer rather than its own lists. - proc.p_filemon is added which is protected by PROC_LOCK. This improves performance and avoids double-fork issues, taking allproc_lock while in syscalls, and walking the process tree in syscalls. A particular proc.p_filemon can only be changed to NULL or another filemon, or the filemon inherited, while the filemon->lock is held. - Filemon are reference counted. On the last reference the log will be closed. - When closing the devfs file handle, the filemon will be detached from all processes and inheritance prevented. - Disallow attaching to a process already being traced since filemon is typically intended to be used on children only. This is allowed for curproc as bmake relies on this behavior for rare cases when combining .MAKE with .META. - Detach any previously tracked process on ioctl(FILEMON_SET_PID). - Handle error from devfs_set_cdevpriv() in filemon_open(). - The global filemon lock and lists are removed. - A free list is no longer kept. Previously this list was forever-expanding and never garbage cleaned. - No longer loses track of double-forks. If the process holding the filemon handle closes it will close the log rather than wait on a daemonized process, but it will log all activity until it closes its handle. The filemon will be removed from the process and not inherited. - A separate process count is kept only as an optimization for forced detachment to avoid taking allproc_lock and walking the entire process tree. - struct filemon access is protected by sx(9) filemon->lock as it was before. - Add more comments and KASSERTS. MFC after: 2 weeks Reviewed by: kib, mjg, markj (all on previous versions) Sponsored by: EMC / Isilon Storage Division Differential Revision: https://reviews.freebsd.org/D5520 Deleted: head/sys/dev/filemon/filemon_lock.c Modified: head/share/man/man4/filemon.4 head/sys/dev/filemon/filemon.c head/sys/dev/filemon/filemon_wrapper.c head/sys/sys/param.h head/sys/sys/proc.h Modified: head/share/man/man4/filemon.4 ============================================================================== --- head/share/man/man4/filemon.4 Mon Mar 21 18:48:20 2016 (r297155) +++ head/share/man/man4/filemon.4 Mon Mar 21 20:29:27 2016 (r297156) @@ -31,7 +31,7 @@ .\" .\" $FreeBSD$ .\" -.Dd March 9, 2016 +.Dd March 21, 2016 .Dt FILEMON 4 .Os .Sh NAME @@ -116,6 +116,10 @@ Each takes a single argument. Write the internal tracing buffer to the supplied open file descriptor. .It Dv FILEMON_SET_PID Child process ID to trace. +This should normally be done under the control of a parent in the child after +.Xr fork 2 +but before anything else. +See the example below. .El .Sh RETURN VALUES .\" .Rv -std ioctl @@ -138,6 +142,20 @@ The .Nm handle is already associated with a file descriptor. .El +.Pp +The +.Fn ioctl +system call +with +.Dv FILEMON_SET_PID +will fail if: +.Bl -tag -width Er +.It Bq Er ESRCH +No process having the specified process ID exists. +.It Bq Er EBUSY +The process ID specified is already being traced and was not the current +process. +.El .Sh FILES .Bl -tag -width ".Pa /dev/filemon" .It Pa /dev/filemon @@ -199,14 +217,5 @@ A 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 . Modified: head/sys/dev/filemon/filemon.c ============================================================================== --- head/sys/dev/filemon/filemon.c Mon Mar 21 18:48:20 2016 (r297155) +++ head/sys/dev/filemon/filemon.c Mon Mar 21 20:29:27 2016 (r297156) @@ -1,7 +1,7 @@ /*- * Copyright (c) 2011, David E. O'Brien. * Copyright (c) 2009-2011, Juniper Networks, Inc. - * Copyright (c) 2015, EMC Corp. + * Copyright (c) 2015-2016, EMC Corp. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -46,7 +46,6 @@ __FBSDID("$FreeBSD$"); #include <sys/module.h> #include <sys/poll.h> #include <sys/proc.h> -#include <sys/queue.h> #include <sys/sx.h> #include <sys/syscall.h> #include <sys/sysent.h> @@ -80,23 +79,126 @@ static struct cdevsw filemon_cdevsw = { 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. */ + u_int refcnt; /* Pointer reference count. */ + u_int proccnt; /* Process 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; +static void filemon_output(struct filemon *filemon, char *msg, size_t len); + +static __inline struct filemon * +filemon_acquire(struct filemon *filemon) +{ + + if (filemon != NULL) + refcount_acquire(&filemon->refcnt); + return (filemon); +} + +/* + * Release a reference and on the last one write the footer and free the + * filemon. + */ +static void +filemon_release(struct filemon *filemon) +{ + size_t len; + struct timeval now; + + if (refcount_release(&filemon->refcnt) == 0) + return; + /* + * There are valid cases of releasing while locked, such as in + * filemon_untrack_processes, but none which are done where there + * is not at least 1 reference remaining. + */ + sx_assert(&filemon->lock, SA_UNLOCKED); + + 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); +} + +/* + * 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); +} + +/* Remove and release the filemon on the given process. */ +static void +filemon_proc_drop(struct proc *p) +{ + struct filemon *filemon; + + 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 = p->p_filemon; + p->p_filemon = NULL; + --filemon->proccnt; + PROC_UNLOCK(p); + /* + * This should not be the last reference yet. filemon_release() + * cannot be called with filemon locked, which the caller expects + * will stay locked. + */ + KASSERT(filemon->refcnt > 1, ("%s: proc %p dropping filemon %p " + "with last reference", __func__, p, filemon)); + filemon_release(filemon); +} + +/* Unlock and release the filemon. */ +static __inline void +filemon_drop(struct filemon *filemon) +{ + + sx_xunlock(&filemon->lock); + filemon_release(filemon); +} -#include "filemon_lock.c" #include "filemon_wrapper.c" static void @@ -115,35 +217,120 @@ filemon_comment(struct filemon *filemon) filemon_output(filemon, filemon->msgbufr, len); } +/* + * Invalidate the passed filemon in all processes. + */ static void -filemon_dtr(void *data) +filemon_untrack_processes(struct filemon *filemon) { - struct filemon *filemon = data; + 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); + /* Avoid allproc loop if there is no need. */ + if (filemon->proccnt == 0) + return; + + /* + * 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); - /* Remove from the in-use list. */ - TAILQ_REMOVE(&filemons_inuse, filemon, link); + /* + * It's possible some references were acquired but will be + * dropped shortly as they are restricted from being + * inherited. There is at least the reference in cdevpriv remaining. + */ + KASSERT(filemon->refcnt > 0, ("%s: filemon %p should have " + "references still.", __func__, filemon)); + KASSERT(filemon->proccnt == 0, ("%s: filemon %p should not have " + "attached procs still.", __func__, filemon)); +} - fp = filemon->fp; - filemon->fp = NULL; - filemon->p = NULL; - /* Add to the free list. */ - TAILQ_INSERT_TAIL(&filemons_free, filemon, link); +/* The devfs file is being closed. Untrace all processes. */ +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; + + sx_xlock(&filemon->lock); + /* + * Detach the filemon. The actual closing of it may not + * occur until syscalls in other threads with references complete. + * The filemon cannot be inherited after this though. + */ + filemon_untrack_processes(filemon); + filemon_drop(filemon); +} + +/* Attach the filemon to the process. */ +static int +filemon_attach_proc(struct filemon *filemon, struct proc *p) +{ + struct filemon *filemon2; - if (fp != NULL) - fdrop(fp, curthread); + 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); + /* + * Don't allow truncating other process traces. It is + * not really intended to trace procs other than curproc + * anyhow. + */ + if (p->p_filemon != NULL && p != curproc) + return (EBUSY); + /* + * Historic behavior of filemon has been to let a child initiate + * tracing on itself and cease existing tracing. Bmake + * .META + .MAKE relies on this. It is only relevant for attaching to + * curproc. + */ + while (p->p_filemon != NULL) { + 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. + */ } + + KASSERT(p->p_filemon == NULL, + ("%s: proc %p didn't detach filemon %p", __func__, p, + p->p_filemon)); + p->p_filemon = filemon_acquire(filemon); + ++filemon->proccnt; + + return (0); } static int @@ -178,10 +365,16 @@ filemon_ioctl(struct cdev *dev, u_long c /* 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; @@ -199,35 +392,19 @@ static int 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(); - - if ((filemon = TAILQ_FIRST(&filemons_free)) != NULL) - TAILQ_REMOVE(&filemons_free, filemon, link); + filemon = malloc(sizeof(*filemon), M_FILEMON, + M_WAITOK | M_ZERO); + sx_init(&filemon->lock, "filemon"); + refcount_init(&filemon->refcnt, 1); + + error = devfs_set_cdevpriv(filemon, filemon_dtr); + if (error != 0) + filemon_release(filemon); - /* 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"); - } - - 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 @@ -241,7 +418,6 @@ filemon_close(struct cdev *dev __unused, static void filemon_load(void *dummy __unused) { - sx_init(&access_lock, "filemons_inuse"); /* Install the syscall wrappers. */ filemon_wrapper_install(); @@ -253,38 +429,11 @@ filemon_load(void *dummy __unused) 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 Modified: head/sys/dev/filemon/filemon_wrapper.c ============================================================================== --- head/sys/dev/filemon/filemon_wrapper.c Mon Mar 21 18:48:20 2016 (r297155) +++ head/sys/dev/filemon/filemon_wrapper.c Mon Mar 21 20:29:27 2016 (r297156) @@ -1,7 +1,7 @@ /*- * Copyright (c) 2011, David E. O'Brien. * Copyright (c) 2009-2011, Juniper Networks, Inc. - * Copyright (c) 2015, EMC Corp. + * Copyright (c) 2015-2016, EMC Corp. * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -65,33 +65,6 @@ filemon_output(struct filemon *filemon, 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 @@ filemon_wrapper_chdir(struct thread *td, 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_wrapper_chdir(struct thread *td, filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -126,7 +99,7 @@ filemon_event_process_exec(void *arg __u char *fullpath, *freepath; size_t len; - if ((filemon = filemon_pid_check(p)) != NULL) { + if ((filemon = filemon_proc_get(p)) != NULL) { fullpath = "<unknown>"; freepath = NULL; @@ -139,7 +112,7 @@ filemon_event_process_exec(void *arg __u filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); free(freepath, M_TEMP); } @@ -154,7 +127,7 @@ filemon_wrapper_open(struct thread *td, 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 @@ filemon_wrapper_open(struct thread *td, curproc->p_pid, filemon->fname1); filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -193,7 +166,7 @@ filemon_wrapper_openat(struct thread *td 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 @@ filemon_wrapper_openat(struct thread *td curproc->p_pid, filemon->fname2, filemon->fname1); filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -245,7 +218,7 @@ filemon_wrapper_rename(struct thread *td 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_wrapper_rename(struct thread *td filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -273,7 +246,7 @@ filemon_wrapper_link(struct thread *td, 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_wrapper_link(struct thread *td, filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -301,7 +274,7 @@ filemon_wrapper_symlink(struct thread *t 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_wrapper_symlink(struct thread *t filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -329,7 +302,7 @@ filemon_wrapper_linkat(struct thread *td 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_wrapper_linkat(struct thread *td filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -357,7 +330,7 @@ filemon_wrapper_stat(struct thread *td, 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_wrapper_stat(struct thread *td, filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -385,7 +358,7 @@ filemon_wrapper_freebsd32_stat(struct th 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_wrapper_freebsd32_stat(struct th filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -408,29 +381,25 @@ filemon_event_process_exit(void *arg __u { 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. + */ + KASSERT(p->p_filemon == NULL || p->p_filemon == filemon, + ("%s: p %p was attached while exiting, expected " + "filemon %p or NULL", __func__, p, filemon)); + if (p->p_filemon == filemon) + filemon_proc_drop(p); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -443,7 +412,7 @@ filemon_wrapper_unlink(struct thread *td 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 +422,7 @@ filemon_wrapper_unlink(struct thread *td filemon_output(filemon, filemon->msgbufr, len); - sx_xunlock(&filemon->lock); + filemon_drop(filemon); } } @@ -467,14 +436,34 @@ filemon_event_process_fork(void *arg __u 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); + ++filemon->proccnt; + } + PROC_UNLOCK(p2); + } + + filemon_drop(filemon); } } Modified: head/sys/sys/param.h ============================================================================== --- head/sys/sys/param.h Mon Mar 21 18:48:20 2016 (r297155) +++ head/sys/sys/param.h Mon Mar 21 20:29:27 2016 (r297156) @@ -58,7 +58,7 @@ * in the range 5 to 9. */ #undef __FreeBSD_version -#define __FreeBSD_version 1100103 /* Master, propagated to newvers */ +#define __FreeBSD_version 1100104 /* Master, propagated to newvers */ /* * __FreeBSD_kernel__ indicates that this system uses the kernel of FreeBSD, Modified: head/sys/sys/proc.h ============================================================================== --- head/sys/sys/proc.h Mon Mar 21 18:48:20 2016 (r297155) +++ head/sys/sys/proc.h Mon Mar 21 20:29:27 2016 (r297156) @@ -162,6 +162,7 @@ struct pargs { */ struct cpuset; struct filecaps; +struct filemon; struct kaioinfo; struct kaudit_record; struct kdtrace_proc; @@ -580,6 +581,7 @@ struct proc { 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201603212029.u2LKTSJp010970>