Re: In-kernel process exit hooks?

[Paul Goyette <paul%whooppee.com@localhost>]

> > Another possibility would be to change filemon(4) to do fd_getfile
> > each it needs to use the file descriptor.  This makes it a little
> > more brittle (fails if you close the descriptor), but would sidestep
> > the problem.
>
> Hmmm, perhaps.  Failure would not be a problem, since we would just
> revert to the initial "output file unspecified" conditions.
>
> I think I like this approach.  :)  I'll give it a try.

This actually works quite well.  Please see the attached diffs for your 
review.

One possible problem is what happens if the monitoring program closes 
the file descriptor, and then re-uses that fd?  I've included a check to 
compare the original 'struct file *' pointer with the current one, which 
will catch "some" instances, but not guaranteed to catch them all.  It 
could be a bit of a surprise if filemon output shows up in unexpected 
places.  :)

Because of this potential for surprising the user, I think I'm still 
leaning to my earlier proposal of extending exithook processing.  But 
given the limited number of use-cases for filemon, I could live with 
making the fd_getfile()-only-when-you-need-it change instead.


+------------------+--------------------------+------------------------+
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:      |
| (Retired)        | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com   |
| Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org |
+------------------+--------------------------+------------------------+

Index: filemon.c
===================================================================
RCS file: /cvsroot/src/sys/dev/filemon/filemon.c,v
retrieving revision 1.24
diff -u -p -r1.24 filemon.c
--- filemon.c	5 Jan 2016 22:08:54 -0000	1.24
+++ filemon.c	7 Jan 2016 00:45:46 -0000
@@ -94,9 +94,21 @@ filemon_output(struct filemon * filemon,
 {
 	struct uio auio;
 	struct iovec aiov;
+	struct file *fp;     /* Output file pointer. */
 
-	if (filemon->fm_fp == NULL)
+
+	if (filemon->fm_fp == NULL)	/* No output file specified */
+		return;
+	if ((fp = fd_getfile(filemon->fm_fd)) != filemon->fm_fp) {
+		/*
+		 * The file descriptor refers to a different file
+		 * than when it was passed to SET_FD.  So assume we
+		 * never had an output file.
+		 */
+		filemon->fm_fp = NULL;
+		filemon->fm_fd = -1;
 		return;
+	}
 
 	aiov.iov_base = msg;
 	aiov.iov_len = len;
@@ -122,6 +134,7 @@ filemon_output(struct filemon * filemon,
 	(*filemon->fm_fp->f_ops->fo_write) (filemon->fm_fp,
 	    &(filemon->fm_fp->f_offset),
 	    &auio, curlwp->l_cred, FOF_UPDATE_OFFSET);
+	fd_putfile(filemon->fm_fd);	/* release our reference */
 }
 
 void
@@ -264,10 +277,7 @@ filemon_close(struct file * fp)
 	 * once we have exclusive access, it should never be used again
 	 */
 	rw_enter(&filemon->fm_mtx, RW_WRITER);
-	if (filemon->fm_fp) {
-		fd_putfile(filemon->fm_fd);	/* release our reference */
-		filemon->fm_fp = NULL;
-	}
+	filemon->fm_fp = NULL;
 	rw_exit(&filemon->fm_mtx);
 	rw_destroy(&filemon->fm_mtx);
 	kmem_free(filemon, sizeof(struct filemon));
@@ -309,6 +319,7 @@ filemon_ioctl(struct file * fp, u_long c
 			error = EBADF;
 			break;
 		}
+		fd_putfile(filemon->fm_fd);
 		/* Write the file header. */
 		filemon_comment(filemon);
 		break;