perhaps it made art's head hurt because it is wrong. triggers NULL

deref in sensor_task_work(); found by krw, dlg should talk to him

1 files changed, 50 insertions, 65 deletions

diff --git a/sys/kern/kern_sensors.c b/sys/kern/kern_sensors.c
index 1870c631862..798cd7789b5 100644
--- a/sys/kern/kern_sensors.c
+++ b/sys/kern/kern_sensors.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_sensors.c,v 1.18 2007/06/01 04:15:45 dlg Exp $	*/
+/*	$OpenBSD: kern_sensors.c,v 1.19 2007/06/04 18:42:05 deraadt Exp $	*/
 
 /*
  * Copyright (c) 2005 David Gwynne <dlg@openbsd.org>
@@ -27,7 +27,6 @@
 #include <sys/time.h>
 #include <sys/device.h>
 #include <sys/hotplug.h>
-#include <sys/workq.h>
 
 #include <sys/sensors.h>
 #include "hotplug.h"
@@ -36,21 +35,20 @@ int			sensordev_count = 0;
 SLIST_HEAD(, ksensordev) sensordev_list = SLIST_HEAD_INITIALIZER(sensordev_list);
 
 struct sensor_task {
-	void				(*func)(void *);
 	void				*arg;
+	void				(*func)(void *);
 
 	int				period;
 	time_t				nextrun;
+	volatile int			running;
 	TAILQ_ENTRY(sensor_task)	entry;
 };
 
-struct timeout sensor_task_to;
-TAILQ_HEAD(, sensor_task) sensor_task_list =
-    TAILQ_HEAD_INITIALIZER(sensor_task_list);
-
-void	sensor_task_tick(void *);
+void	sensor_task_create(void *);
+void	sensor_task_thread(void *);
 void	sensor_task_schedule(struct sensor_task *);
-void	sensor_task_work(void *, void *);
+
+TAILQ_HEAD(, sensor_task) tasklist = TAILQ_HEAD_INITIALIZER(tasklist);
 
 void
 sensordev_install(struct ksensordev *sensdev)
@@ -190,16 +188,15 @@ sensor_task_register(void *arg, void (*func)(void *), int period)
 	st->arg = arg;
 	st->func = func;
 	st->period = period;
-	st->nextrun = 0;
 
-	if (TAILQ_EMPTY(&sensor_task_list))
-		timeout_set(&sensor_task_to, sensor_task_tick, NULL);
-	else
-		timeout_del(&sensor_task_to);
+	st->running = 1;
 
-	TAILQ_INSERT_HEAD(&sensor_task_list, st, entry);
+	if (TAILQ_EMPTY(&tasklist))
+		kthread_create_deferred(sensor_task_create, NULL);
 
-	sensor_task_tick(NULL);
+	st->nextrun = 0;
+	TAILQ_INSERT_HEAD(&tasklist, st, entry);
+	wakeup(&tasklist);
 
 	return (0);
 }
@@ -207,78 +204,65 @@ sensor_task_register(void *arg, void (*func)(void *), int period)
 void
 sensor_task_unregister(void *arg)
 {
-	struct sensor_task	*st, *nst;
-
-	timeout_del(&sensor_task_to);
-
-	nst = TAILQ_FIRST(&sensor_task_list);
-	while (nst != NULL) {
-		st = nst;
-		nst = TAILQ_NEXT(st, entry);
+	struct sensor_task	*st;
 
+	TAILQ_FOREACH(st, &tasklist, entry) {
 		if (st->arg == arg)
-			free(st, M_DEVBUF);
+			st->running = 0;
 	}
+}
 
-	if (TAILQ_EMPTY(&sensor_task_list))
-		return;
-
-	sensor_task_tick(NULL);
+void
+sensor_task_create(void *arg)
+{
+	if (kthread_create(sensor_task_thread, NULL, NULL, "sensors") != 0)
+		panic("sensors kthread");
 }
 
 void
-sensor_task_tick(void *arg)
+sensor_task_thread(void *arg)
 {
 	struct sensor_task	*st, *nst;
-	time_t			now = time_uptime;
+	time_t			now;
 
-#ifdef DIAGNOSTIC
-	if (TAILQ_EMPTY(&sensor_task_list))
-		panic("sensor task tick for no sensors");
-#endif
+	while (!TAILQ_EMPTY(&tasklist)) {
+		while ((nst = TAILQ_FIRST(&tasklist))->nextrun >
+		    (now = time_uptime))
+			tsleep(&tasklist, PWAIT, "timeout",
+			    (nst->nextrun - now) * hz);
 
-	nst = TAILQ_FIRST(&sensor_task_list);
-	while (nst->nextrun <= now) {
-		st = nst;
-		nst = TAILQ_NEXT(st, entry);
+		while ((st = nst) != NULL) {
+			nst = TAILQ_NEXT(st, entry);
 
-		/* try to schedule the task */
-		if (workq_add_task(NULL, 0, sensor_task_work, st, NULL) != 0) {
-			timeout_add(&sensor_task_to, hz);
-			return;
-		}
-
-		/* take it out while we work on it */
-		TAILQ_REMOVE(&sensor_task_list, st, entry);
-
-		if (nst == NULL)
-			return;
-	}
-
-	timeout_add(&sensor_task_to, (nst->nextrun - now) * hz);
-}
+			if (st->nextrun > now)
+				break;
 
-void
-sensor_task_work(void *xst, void *arg)
-{
-	struct sensor_task	*st = xst;
+			/* take it out while we work on it */
+			TAILQ_REMOVE(&tasklist, st, entry);
 
-	timeout_del(&sensor_task_to);
+			if (!st->running) {
+				free(st, M_DEVBUF);
+				continue;
+			}
 
-	st->func(st->arg);
-	sensor_task_schedule(st);
+			/* run the task */
+			st->func(st->arg);
+			/* stick it back in the tasklist */
+			sensor_task_schedule(st);
+		}
+	}
 
-	sensor_task_tick(NULL);
+	kthread_exit(0);
 }
 
 void
 sensor_task_schedule(struct sensor_task *st)
 {
-	struct sensor_task	*cst;
+	struct sensor_task 	*cst;
 
 	st->nextrun = time_uptime + st->period;
 
-	TAILQ_FOREACH(cst, &sensor_task_list, entry) {
+	TAILQ_FOREACH(cst, &tasklist, entry) {
 		if (cst->nextrun > st->nextrun) {
 			TAILQ_INSERT_BEFORE(cst, st, entry);
 			return;
@@ -286,5 +270,6 @@ sensor_task_schedule(struct sensor_task *st)
 	}
 
 	/* must be an empty list, or at the end of the list */
-	TAILQ_INSERT_TAIL(&sensor_task_list, st, entry);
+	TAILQ_INSERT_TAIL(&tasklist, st, entry);
 }
+