The saga continues! :)
I previously posted one set of patches that changes filemon to use
fd_{get,put}file() only when the activity-log file is actually being
accessed. This works, but doesn't prevent or detect when the
application program closes-and-reopens that fd.
Attached to this Email is a separate patch, using exithooks. The
current exithook implementation is extended to have an additional
"phase" argument, with values of EXITHOOK_BEFORE_CLOSE and
EXITHOOK_AFTER_CLOSE. (Existing uses of exithooks in sys_aio, sysv_sem,
and rump are updated to use EXITHOOK_AFTER_CLOSE.)
Filemon(4) now establishes an EXITHOOK_BEFORE_CLOSE which searches the
process's file descriptor table for entries to /dev/filemon. If any are
found, the associated activity-log file is disassociated from the
filemon fd and the extra reference is released.
(Note that establishing the exithook is actually done through a config
finalizer routine, since the built-in MODULE_CLASS_DRIVER modules are
initialized before the exithook stuff in exec_init().)
This exithook approach also works. There is still one problem, however.
If the application program attempts to explicitly close the log-file fd
while it is still associated with the filemon, we get the same hang.
Next, I'm going to have a look at fd_getfile2()/fclose() and see if that
is a viable approach.
+------------------+--------------------------+------------------------+
| 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: dev/filemon/filemon.c
===================================================================
RCS file: /cvsroot/src/sys/dev/filemon/filemon.c,v
retrieving revision 1.24
diff -u -p -r1.24 filemon.c
--- dev/filemon/filemon.c 5 Jan 2016 22:08:54 -0000 1.24
+++ dev/filemon/filemon.c 7 Jan 2016 23:54:37 -0000
@@ -43,6 +43,8 @@ __KERNEL_RCSID(0, "$NetBSD: filemon.c,v
#include <sys/kmem.h>
#include <sys/syslog.h>
#include <sys/kauth.h>
+#include <sys/device.h>
+#include <sys/errno.h>
#include "filemon.h"
#include "ioconf.h"
@@ -89,6 +91,75 @@ static TAILQ_HEAD(, filemon) filemons_in
static int logLevel = LOG_DEBUG;
#endif
+/*
+ * Use an exithook to make sure that the filemon device is closed and
+ * our additional reference to the output file is released before the
+ * normal closure of open files. The extra reference would otherwise
+ * cause process exit to hang indefinitely.
+ *
+ * We need to use a config_finalize() routine to register the hook
+ * due to system initialization sequence.
+ */
+void *hook = NULL; /* cookie from exithook_establish() */
+
+static int filemon_register_hook(device_t dev __unused);
+static void filemon_exithook(struct proc *p, void *arg);
+
+static int
+filemon_register_hook(device_t dev __unused)
+{
+
+ if (hook == NULL)
+ hook = exithook_establish(filemon_exithook, hook,
+ EXITHOOK_BEFORE_CLOSE);
+printf("%s: hook %p\n", __func__, hook);
+ if (hook != NULL)
+ return 0;
+ return ENXIO;
+}
+
+static void
+filemon_exithook(struct proc *p, void *arg)
+{
+ int fd;
+ struct filemon *filemon;
+ fdfile_t *ff;
+ filedesc_t *fdp;
+ fdtab_t *dt;
+
+ fdp = p->p_fd;
+ dt = fdp->fd_dt;
+ for (fd = 0; fd < dt->dt_nfiles; fd++) {
+ ff = dt->dt_ff[fd];
+ KASSERT(fd >= NDFDFILE ||
+ ff == (fdfile_t *)fdp->fd_dfdfile[fd]);
+ if (ff == NULL || ff->ff_file == NULL)
+ continue;
+ if (ff->ff_file->f_type != DTYPE_MISC ||
+ ff->ff_file->f_ops != &filemon_fileops)
+ continue;
+printf("%s: found pid %d fd %d\n", __func__, p->p_pid, fd);
+ rw_enter(&filemon_mtx, RW_WRITER);
+ filemon = ff->ff_file->f_data;
+ if (filemon != NULL) {
+
+ /*
+ * If we have an output file, release our
+ * reference to it.
+ */
+ rw_enter(&filemon->fm_mtx, RW_WRITER);
+ if (filemon->fm_fp) {
+printf("%s: releasing %d\n", __func__, filemon->fm_fd);
+ KASSERT(p == curproc);
+ fd_putfile(filemon->fm_fd);
+ filemon->fm_fp = NULL;
+ }
+ rw_exit(&filemon->fm_mtx);
+ }
+ rw_exit(&filemon_mtx);
+ }
+}
+
void
filemon_output(struct filemon * filemon, char *msg, size_t len)
{
@@ -216,6 +287,9 @@ filemon_open(dev_t dev, int oflags __unu
struct file *fp;
int error, fd;
+ if (hook == NULL)
+ return ENXIO;
+
/* falloc() will fill in the descriptor for us. */
if ((error = fd_allocfile(&fp, &fd)) != 0)
return error;
@@ -265,6 +339,7 @@ filemon_close(struct file * fp)
*/
rw_enter(&filemon->fm_mtx, RW_WRITER);
if (filemon->fm_fp) {
+printf("%s: releasing %d\n", __func__, filemon->fm_fd);
fd_putfile(filemon->fm_fd); /* release our reference */
filemon->fm_fp = NULL;
}
@@ -399,19 +474,35 @@ filemon_modcmd(modcmd_t cmd, void *data)
switch (cmd) {
case MODULE_CMD_INIT:
+ KASSERT(hook == NULL);
#ifdef DEBUG
logLevel = LOG_INFO;
#endif
error = filemon_load(data);
+ if (error != 0)
+ break;
#ifdef _MODULE
- if (!error)
- error = devsw_attach("filemon", NULL, &bmajor,
- &filemon_cdevsw, &cmajor);
+ error = devsw_attach("filemon", NULL, &bmajor,
+ &filemon_cdevsw, &cmajor);
+ if (error != 0) {
+ filemon_unload();
+ break;
+ }
#endif
+ error = config_finalize_register(NULL, filemon_register_hook);
+ if (error != 0) {
+ devsw_detach(NULL, &filemon_cdevsw);
+ filemon_unload();
+ break;
+ }
break;
case MODULE_CMD_FINI:
+ if (hook != NULL) {
+ exithook_disestablish(hook, EXITHOOK_BEFORE_CLOSE);
+ hook = NULL;
+ }
error = filemon_unload();
#ifdef _MODULE
if (!error)
Index: kern/kern_exit.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_exit.c,v
retrieving revision 1.248
diff -u -p -r1.248 kern_exit.c
--- kern/kern_exit.c 13 Oct 2015 06:47:21 -0000 1.248
+++ kern/kern_exit.c 7 Jan 2016 23:55:02 -0000
@@ -279,10 +279,11 @@ exit1(struct lwp *l, int rv)
* Close open files, release open-file table and free signal
* actions. This may block!
*/
+ doexithooks(p, EXITHOOK_BEFORE_CLOSE);
fd_free();
cwdfree(p->p_cwdi);
p->p_cwdi = NULL;
- doexithooks(p);
+ doexithooks(p, EXITHOOK_AFTER_CLOSE);
sigactsfree(p->p_sigacts);
/*
Index: kern/kern_hook.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_hook.c,v
retrieving revision 1.6
diff -u -p -r1.6 kern_hook.c
--- kern/kern_hook.c 22 Nov 2013 21:04:11 -0000 1.6
+++ kern/kern_hook.c 7 Jan 2016 23:55:13 -0000
@@ -219,26 +219,33 @@ doexechooks(struct proc *p)
hook_proc_run(&exechook_list, p);
}
-static hook_list_t exithook_list = LIST_HEAD_INITIALIZER(exithook_list);
+static hook_list_t exithook_list0 = LIST_HEAD_INITIALIZER(exithook_list0);
+static hook_list_t exithook_list1 = LIST_HEAD_INITIALIZER(exithook_list1);
+static hook_list_t *exithook_table[] = {
+ &exithook_list0, /* EXITHOOK_BEFORE_CLOSE */
+ &exithook_list1, /* EXITHOOK_AFTER_CLOSE */
+};
extern krwlock_t exec_lock;
void *
-exithook_establish(void (*fn)(struct proc *, void *), void *arg)
+exithook_establish(void (*fn)(struct proc *, void *), void *arg, int ph)
{
void *rv;
+ KASSERT(ph >= 0 && ph < __arraycount(exithook_table));
rw_enter(&exec_lock, RW_WRITER);
- rv = hook_establish(&exithook_list, (void (*)(void *))fn, arg);
+ rv = hook_establish(exithook_table[ph], (void (*)(void *))fn, arg);
rw_exit(&exec_lock);
return rv;
}
void
-exithook_disestablish(void *vhook)
+exithook_disestablish(void *vhook, int ph)
{
+ KASSERT(ph >= 0 && ph < __arraycount(exithook_table));
rw_enter(&exec_lock, RW_WRITER);
- hook_disestablish(&exithook_list, vhook);
+ hook_disestablish(exithook_table[ph], vhook);
rw_exit(&exec_lock);
}
@@ -246,9 +253,11 @@ exithook_disestablish(void *vhook)
* Run exit hooks.
*/
void
-doexithooks(struct proc *p)
+doexithooks(struct proc *p, int ph)
{
- hook_proc_run(&exithook_list, p);
+
+ KASSERT(ph >= 0 && ph < __arraycount(exithook_table));
+ hook_proc_run(exithook_table[ph], p);
}
static hook_list_t forkhook_list = LIST_HEAD_INITIALIZER(forkhook_list);
Index: kern/sys_aio.c
===================================================================
RCS file: /cvsroot/src/sys/kern/sys_aio.c,v
retrieving revision 1.40
diff -u -p -r1.40 sys_aio.c
--- kern/sys_aio.c 5 Sep 2014 09:20:59 -0000 1.40
+++ kern/sys_aio.c 7 Jan 2016 23:55:29 -0000
@@ -131,7 +131,7 @@ aio_fini(bool interface)
sysctl_teardown(&aio_sysctl);
KASSERT(aio_jobs_count == 0);
- exithook_disestablish(aio_ehook);
+ exithook_disestablish(aio_ehook, EXITHOOK_AFTER_CLOSE);
pool_destroy(&aio_job_pool);
pool_destroy(&aio_lio_pool);
return 0;
@@ -149,7 +149,7 @@ aio_init(void)
"aio_jobs_pool", &pool_allocator_nointr, IPL_NONE);
pool_init(&aio_lio_pool, sizeof(struct lio_req), 0, 0, 0,
"aio_lio_pool", &pool_allocator_nointr, IPL_NONE);
- aio_ehook = exithook_establish(aio_exit, NULL);
+ aio_ehook = exithook_establish(aio_exit, NULL, EXITHOOK_AFTER_CLOSE);
error = sysctl_aio_init();
if (error != 0) {
Index: kern/sysv_sem.c
===================================================================
RCS file: /cvsroot/src/sys/kern/sysv_sem.c,v
retrieving revision 1.95
diff -u -p -r1.95 sysv_sem.c
--- kern/sysv_sem.c 6 Nov 2015 02:26:42 -0000 1.95
+++ kern/sysv_sem.c 7 Jan 2016 23:55:46 -0000
@@ -153,7 +153,7 @@ static int
seminit_exithook(void)
{
- hook = exithook_establish(semexit, NULL);
+ hook = exithook_establish(semexit, NULL, EXITHOOK_AFTER_CLOSE);
return 0;
}
@@ -172,7 +172,7 @@ semfini(void)
/* Remove the exit hook */
if (hook)
- exithook_disestablish(hook);
+ exithook_disestablish(hook, EXITHOOK_AFTER_CLOSE);
/* Destroy all our condvars */
for (i = 0; i < seminfo.semmni; i++) {
Index: rump/librump/rumpkern/lwproc.c
===================================================================
RCS file: /cvsroot/src/sys/rump/librump/rumpkern/lwproc.c,v
retrieving revision 1.35
diff -u -p -r1.35 lwproc.c
--- rump/librump/rumpkern/lwproc.c 18 Apr 2015 15:49:18 -0000 1.35
+++ rump/librump/rumpkern/lwproc.c 7 Jan 2016 23:56:08 -0000
@@ -117,7 +117,8 @@ lwproc_proc_free(struct proc *p)
chgproccnt(kauth_cred_getuid(cred), -1);
rump_proc_vfs_release(p);
- doexithooks(p);
+ doexithooks(p, EXITHOOK_BEFORE_CLOSE);
+ doexithooks(p, EXITHOOK_AFTER_CLOSE);
lim_free(p->p_limit);
pstatsfree(p->p_stats);
kauth_cred_free(p->p_cred);
Index: sys/systm.h
===================================================================
RCS file: /cvsroot/src/sys/sys/systm.h,v
retrieving revision 1.269
diff -u -p -r1.269 systm.h
--- sys/systm.h 29 Oct 2015 00:27:08 -0000 1.269
+++ sys/systm.h 7 Jan 2016 23:56:23 -0000
@@ -375,9 +375,11 @@ void doexechooks(struct proc *);
/*
* Exit hooks. Subsystems may want to do cleanup when a process exits.
*/
-void *exithook_establish(void (*)(struct proc *, void *), void *);
-void exithook_disestablish(void *);
-void doexithooks(struct proc *);
+#define EXITHOOK_BEFORE_CLOSE 0
+#define EXITHOOK_AFTER_CLOSE 1
+void *exithook_establish(void (*)(struct proc *, void *), void *, int);
+void exithook_disestablish(void *, int);
+void doexithooks(struct proc *, int);
/*
* Fork hooks. Subsystems may want to do special processing when a process
