> 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;
