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

1 files changed, 235 insertions, 86 deletions

diff --git a/sys/dev/filemon/filemon.c b/sys/dev/filemon/filemon.c
index cd40c5a2e329..3a472f6342fd 100644
--- a/sys/dev/filemon/filemon.c
+++ b/sys/dev/filemon/filemon.c
@@ -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;
 
-		/* 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);
+
+	/*
+	 * 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;
 
-		if (fp != NULL)
-			fdrop(fp, curthread);
+	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;
+
+	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 cmd, caddr_t data, int flag __unused,
 
 	/* 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);
 
-	/* Give up write access. */
-	filemon_unlock_write();
+	error = devfs_set_cdevpriv(filemon, filemon_dtr);
+	if (error != 0)
+		filemon_release(filemon);
 
-	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, int flag __unused, int fmt __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();
 
-	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);
-	}
+	destroy_dev(filemon_dev);
+	filemon_wrapper_deinstall();
 
-	return (error);
+	return (0);
 }
 
 static int