On 2014/03/26 07:30, Mateusz Guzik wrote:
Author: mjg
Date: Tue Mar 25 23:30:35 2014
New Revision: 263755
URL: http://svnweb.freebsd.org/changeset/base/263755
Log:
Document a known problem with handling the process intended to receive
SIGIO in /dev/devctl.
Suggested by: adrian
MFC after: 6 days
Modified:
head/sys/kern/subr_bus.c
Modified: head/sys/kern/subr_bus.c
==============================================================================
--- head/sys/kern/subr_bus.c Tue Mar 25 23:19:45 2014 (r263754)
+++ head/sys/kern/subr_bus.c Tue Mar 25 23:30:35 2014 (r263755)
@@ -490,6 +490,21 @@ devioctl(struct cdev *dev, u_long cmd, c
devsoftc.nonblock = 0;
return (0);
case FIOASYNC:
+ /*
+ * FIXME:
+ * Since this is a simple assignment there is no guarantee that
+ * devsoftc.async_proc consumers will get a valid pointer.
+ *
+ * Example scenario where things break (processes A and B):
+ * 1. A opens devctl
+ * 2. A sends fd to B
+ * 3. B sets itself as async_proc
+ * 4. B exits
+ *
+ * However, normally this requires root privileges and the only
+ * in-tree consumer does not behave in a dangerous way so the
+ * issue is not critical.
+ */
if (*(int*)data)
devsoftc.async_proc = td->td_proc;
else
@@ -575,6 +590,7 @@ devctl_queue_data_f(char *data, int flag
cv_broadcast(&devsoftc.cv);
mtx_unlock(&devsoftc.mtx);
selwakeup(&devsoftc.sel);
+ /* XXX see a comment in devioctl */
p = devsoftc.async_proc;
if (p != NULL) {
PROC_LOCK(p);
I think the async process pointer can be cleared when a process exits
by registering an event handler. please see attached patch.
# HG changeset patch
# Parent 53b614ff2cae108f27e4475989d3a86997017268
diff -r 53b614ff2cae sys/kern/subr_bus.c
--- a/sys/kern/subr_bus.c Thu Mar 27 10:03:50 2014 +0800
+++ b/sys/kern/subr_bus.c Thu Mar 27 15:38:40 2014 +0800
@@ -54,6 +54,7 @@
#include <sys/uio.h>
#include <sys/bus.h>
#include <sys/interrupt.h>
+#include <sys/eventhandler.h>
#include <net/vnet.h>
@@ -399,6 +400,18 @@
} devsoftc;
static struct cdev *devctl_dev;
+static eventhandler_tag devctl_eh;
+
+static void
+devctl_proc_clear(void *arg, struct proc *p, struct image_params *imgp __unused)
+{
+ if (devsoftc.async_proc == p) {
+ mtx_lock(&devsoftc.mtx);
+ if (devsoftc.async_proc == p)
+ devsoftc.async_proc = NULL;
+ mtx_unlock(&devsoftc.mtx);
+ }
+}
static void
devinit(void)
@@ -408,6 +421,8 @@
mtx_init(&devsoftc.mtx, "dev mtx", "devd", MTX_DEF);
cv_init(&devsoftc.cv, "dev cv");
TAILQ_INIT(&devsoftc.devq);
+ devctl_eh = EVENTHANDLER_REGISTER(process_exit, devctl_proc_clear, NULL,
+ EVENTHANDLER_PRI_ANY);
}
static int
@@ -490,25 +505,12 @@
devsoftc.nonblock = 0;
return (0);
case FIOASYNC:
- /*
- * FIXME:
- * Since this is a simple assignment there is no guarantee that
- * devsoftc.async_proc consumers will get a valid pointer.
- *
- * Example scenario where things break (processes A and B):
- * 1. A opens devctl
- * 2. A sends fd to B
- * 3. B sets itself as async_proc
- * 4. B exits
- *
- * However, normally this requires root privileges and the only
- * in-tree consumer does not behave in a dangerous way so the
- * issue is not critical.
- */
+ mtx_lock(&devsoftc.mtx);
if (*(int*)data)
devsoftc.async_proc = td->td_proc;
else
devsoftc.async_proc = NULL;
+ mtx_unlock(&devsoftc.mtx);
return (0);
/* (un)Support for other fcntl() calls. */
@@ -588,15 +590,14 @@
TAILQ_INSERT_TAIL(&devsoftc.devq, n1, dei_link);
devsoftc.queued++;
cv_broadcast(&devsoftc.cv);
- mtx_unlock(&devsoftc.mtx);
selwakeup(&devsoftc.sel);
- /* XXX see a comment in devioctl */
p = devsoftc.async_proc;
if (p != NULL) {
PROC_LOCK(p);
kern_psignal(p, SIGIO);
PROC_UNLOCK(p);
}
+ mtx_unlock(&devsoftc.mtx);
return;
out:
/*
@@ -4503,7 +4504,7 @@
return (0);
case MOD_SHUTDOWN:
- device_shutdown(root_bus);
+ EVENTHANDLER_DEREGISTER(process_exit, devctl_eh);
return (0);
default:
return (EOPNOTSUPP);
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
