MFC r296286,r296470,r296472,r296473,r296575:

  r296286:
    Remove filemon->lock wrappers.
  r296470:
    Only call bwillwrite() for logging to vnodes, as other fo_write() calls do.
  r296472:
    Require kldunload -f to unload.
  r296473:
    Add missing break for r296472.
  r296575:
    FILEMON_SET_FD: Disallow changing the fd.

4 files changed, 49 insertions, 51 deletions

diff --git a/share/man/man4/filemon.4 b/share/man/man4/filemon.4
index fe0c430..92876a3 100644
--- a/share/man/man4/filemon.4
+++ b/share/man/man4/filemon.4
@@ -31,7 +31,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd January 28, 2016
+.Dd March 9, 2016
 .Dt FILEMON 4
 .Os
 .Sh NAME
@@ -125,6 +125,19 @@ function returns the value 0 if successful;
 otherwise the value \-1 is returned and the global variable
 .Va errno
 is set to indicate the error.
+.Sh ERRORS
+The
+.Fn ioctl
+system call
+with
+.Dv FILEMON_SET_FD
+will fail if:
+.Bl -tag -width Er
+.It Bq Er EEXIST
+The
+.Nm
+handle is already associated with a file descriptor.
+.El
 .Sh FILES
 .Bl -tag -width ".Pa /dev/filemon"
 .It Pa /dev/filemon
@@ -193,3 +206,6 @@ 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 .
diff --git a/sys/dev/filemon/filemon.c b/sys/dev/filemon/filemon.c
index 352f682..e33536f 100644
--- a/sys/dev/filemon/filemon.c
+++ b/sys/dev/filemon/filemon.c
@@ -71,8 +71,6 @@ extern struct sysentvec elf64_freebsd_sysvec;
 static d_close_t	filemon_close;
 static d_ioctl_t	filemon_ioctl;
 static d_open_t		filemon_open;
-static int		filemon_unload(void);
-static void		filemon_load(void *);
 
 static struct cdevsw filemon_cdevsw = {
 	.d_version	= D_VERSION,
@@ -130,7 +128,7 @@ filemon_dtr(void *data)
 
 		/* Follow same locking order as filemon_pid_check. */
 		filemon_lock_write();
-		filemon_filemon_lock(filemon);
+		sx_xlock(&filemon->lock);
 
 		/* Remove from the in-use list. */
 		TAILQ_REMOVE(&filemons_inuse, filemon, link);
@@ -143,7 +141,7 @@ filemon_dtr(void *data)
 		TAILQ_INSERT_TAIL(&filemons_free, filemon, link);
 
 		/* Give up write access. */
-		filemon_filemon_unlock(filemon);
+		sx_xunlock(&filemon->lock);
 		filemon_unlock_write();
 
 		if (fp != NULL)
@@ -165,13 +163,15 @@ filemon_ioctl(struct cdev *dev, u_long cmd, caddr_t data, int flag __unused,
 	if ((error = devfs_get_cdevpriv((void **) &filemon)) != 0)
 		return (error);
 
-	filemon_filemon_lock(filemon);
+	sx_xlock(&filemon->lock);
 
 	switch (cmd) {
 	/* Set the output file descriptor. */
 	case FILEMON_SET_FD:
-		if (filemon->fp != NULL)
-			fdrop(filemon->fp, td);
+		if (filemon->fp != NULL) {
+			error = EEXIST;
+			break;
+		}
 
 		error = fget_write(td, *(int *)data,
 #if __FreeBSD_version >= 900041
@@ -198,7 +198,7 @@ filemon_ioctl(struct cdev *dev, u_long cmd, caddr_t data, int flag __unused,
 		break;
 	}
 
-	filemon_filemon_unlock(filemon);
+	sx_xunlock(&filemon->lock);
 	return (error);
 }
 
@@ -308,6 +308,14 @@ filemon_modevent(module_t mod __unused, int type, void *data)
 		error = filemon_unload();
 		break;
 
+	case MOD_QUIESCE:
+		/*
+		 * The wrapper implementation is unsafe for reliable unload.
+		 * Require forcing an unload.
+		 */
+		error = EBUSY;
+		break;
+
 	case MOD_SHUTDOWN:
 		break;
 
diff --git a/sys/dev/filemon/filemon_lock.c b/sys/dev/filemon/filemon_lock.c
index a0347000..5cac47c 100644
--- a/sys/dev/filemon/filemon_lock.c
+++ b/sys/dev/filemon/filemon_lock.c
@@ -29,20 +29,6 @@
 __FBSDID("$FreeBSD$");
 
 static __inline void
-filemon_filemon_lock(struct filemon *filemon)
-{
-
-	sx_xlock(&filemon->lock);
-}
-
-static __inline void
-filemon_filemon_unlock(struct filemon *filemon)
-{
-
-	sx_xunlock(&filemon->lock);
-}
-
-static __inline void
 filemon_lock_read(void)
 {
 
diff --git a/sys/dev/filemon/filemon_wrapper.c b/sys/dev/filemon/filemon_wrapper.c
index 40e09e3..28d34a9 100644
--- a/sys/dev/filemon/filemon_wrapper.c
+++ b/sys/dev/filemon/filemon_wrapper.c
@@ -80,7 +80,8 @@ filemon_output(struct filemon *filemon, char *msg, size_t len)
 	auio.uio_td = curthread;
 	auio.uio_offset = (off_t) -1;
 
-	bwillwrite();
+	if (filemon->fp->f_type == DTYPE_VNODE)
+		bwillwrite();
 
 	fo_write(filemon->fp, &auio, curthread->td_ucred, 0, curthread);
 }
@@ -100,7 +101,7 @@ filemon_pid_check(struct proc *p)
 		TAILQ_FOREACH(filemon, &filemons_inuse, link) {
 			if (p == filemon->p) {
 				sx_sunlock(&proctree_lock);
-				filemon_filemon_lock(filemon);
+				sx_xlock(&filemon->lock);
 				filemon_unlock_read();
 				return (filemon);
 			}
@@ -131,8 +132,7 @@ filemon_wrapper_chdir(struct thread *td, struct chdir_args *uap)
 
 			filemon_output(filemon, filemon->msgbufr, len);
 
-			/* Unlock the found filemon structure. */
-			filemon_filemon_unlock(filemon);
+			sx_xunlock(&filemon->lock);
 		}
 	}
 
@@ -160,8 +160,7 @@ filemon_event_process_exec(void *arg __unused, struct proc *p,
 
 		filemon_output(filemon, filemon->msgbufr, len);
 
-		/* Unlock the found filemon structure. */
-		filemon_filemon_unlock(filemon);
+		sx_xunlock(&filemon->lock);
 
 		free(freepath, M_TEMP);
 	}
@@ -199,8 +198,7 @@ filemon_wrapper_open(struct thread *td, struct open_args *uap)
 			    curproc->p_pid, filemon->fname1);
 			filemon_output(filemon, filemon->msgbufr, len);
 
-			/* Unlock the found filemon structure. */
-			filemon_filemon_unlock(filemon);
+			sx_xunlock(&filemon->lock);
 		}
 	}
 
@@ -252,8 +250,7 @@ filemon_wrapper_openat(struct thread *td, struct openat_args *uap)
 			    curproc->p_pid, filemon->fname2, filemon->fname1);
 			filemon_output(filemon, filemon->msgbufr, len);
 
-			/* Unlock the found filemon structure. */
-			filemon_filemon_unlock(filemon);
+			sx_xunlock(&filemon->lock);
 		}
 	}
 
@@ -281,8 +278,7 @@ filemon_wrapper_rename(struct thread *td, struct rename_args *uap)
 
 			filemon_output(filemon, filemon->msgbufr, len);
 
-			/* Unlock the found filemon structure. */
-			filemon_filemon_unlock(filemon);
+			sx_xunlock(&filemon->lock);
 		}
 	}
 
@@ -310,8 +306,7 @@ filemon_wrapper_link(struct thread *td, struct link_args *uap)
 
 			filemon_output(filemon, filemon->msgbufr, len);
 
-			/* Unlock the found filemon structure. */
-			filemon_filemon_unlock(filemon);
+			sx_xunlock(&filemon->lock);
 		}
 	}
 
@@ -339,8 +334,7 @@ filemon_wrapper_symlink(struct thread *td, struct symlink_args *uap)
 
 			filemon_output(filemon, filemon->msgbufr, len);
 
-			/* Unlock the found filemon structure. */
-			filemon_filemon_unlock(filemon);
+			sx_xunlock(&filemon->lock);
 		}
 	}
 
@@ -369,8 +363,7 @@ filemon_wrapper_linkat(struct thread *td, struct linkat_args *uap)
 
 			filemon_output(filemon, filemon->msgbufr, len);
 
-			/* Unlock the found filemon structure. */
-			filemon_filemon_unlock(filemon);
+			sx_xunlock(&filemon->lock);
 		}
 	}
 
@@ -397,8 +390,7 @@ filemon_wrapper_stat(struct thread *td, struct stat_args *uap)
 
 			filemon_output(filemon, filemon->msgbufr, len);
 
-			/* Unlock the found filemon structure. */
-			filemon_filemon_unlock(filemon);
+			sx_xunlock(&filemon->lock);
 		}
 	}
 
@@ -426,8 +418,7 @@ filemon_wrapper_freebsd32_stat(struct thread *td,
 
 			filemon_output(filemon, filemon->msgbufr, len);
 
-			/* Unlock the found filemon structure. */
-			filemon_filemon_unlock(filemon);
+			sx_xunlock(&filemon->lock);
 		}
 	}
 
@@ -462,8 +453,7 @@ filemon_event_process_exit(void *arg __unused, struct proc *p)
 			filemon->p = NULL;
 		}
 
-		/* Unlock the found filemon structure. */
-		filemon_filemon_unlock(filemon);
+		sx_xunlock(&filemon->lock);
 	}
 }
 
@@ -486,8 +476,7 @@ filemon_wrapper_unlink(struct thread *td, struct unlink_args *uap)
 
 			filemon_output(filemon, filemon->msgbufr, len);
 
-			/* Unlock the found filemon structure. */
-			filemon_filemon_unlock(filemon);
+			sx_xunlock(&filemon->lock);
 		}
 	}
 
@@ -508,8 +497,7 @@ filemon_event_process_fork(void *arg __unused, struct proc *p1,
 
 		filemon_output(filemon, filemon->msgbufr, len);
 
-		/* Unlock the found filemon structure. */
-		filemon_filemon_unlock(filemon);
+		sx_xunlock(&filemon->lock);
 	}
 }