From: miod <miod@openbsd.org>
Date: Thu, 5 Aug 2010 13:50:00 +0000 (+0000)
Subject: Change the management of commands in the active commands TAILQ to let issuers
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=6cd90e55325bf8df342486bb775f37a382ce362f;p=openbsd

Change the management of commands in the active commands TAILQ to let issuers
of synchronous commands perform the TAILQ_REMOVE of the command themselves,
instead of relying upon this being done for us if tsleep() returns zero.

Since we momentarily set `cold' again around suspend, tsleep() becomes a
no-op, which broke this assumption, and in turn caused TAILQ corruption,
with items being put on the freelist while still on the active list.

Found the hard way by ray@ playing with wsmoused after resume.
---

diff --git a/sys/dev/ic/pckbc.c b/sys/dev/ic/pckbc.c
index 710ff9138e0..e83c3fc8158 100644
--- a/sys/dev/ic/pckbc.c
+++ b/sys/dev/ic/pckbc.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: pckbc.c,v 1.23 2010/07/22 14:27:44 deraadt Exp $ */
+/* $OpenBSD: pckbc.c,v 1.24 2010/08/05 13:50:00 miod Exp $ */
 /* $NetBSD: pckbc.c,v 1.5 2000/06/09 04:58:35 soda Exp $ */
 
 /*
@@ -69,7 +69,7 @@ struct pckbc_slotdata {
 	struct pckbc_devcmd cmds[NCMD];
 };
 
-#define CMD_IN_QUEUE(q) (TAILQ_FIRST(&(q)->cmdqueue) != NULL)
+#define CMD_IN_QUEUE(q) (!TAILQ_EMPTY(&(q)->cmdqueue))
 
 void pckbc_init_slotdata(struct pckbc_slotdata *);
 int pckbc_attach_slot(struct pckbc_softc *, pckbc_slot_t);
@@ -796,14 +796,15 @@ pckbc_start(t, slot)
 			if (cmd->status)
 				printf("pckbc_start: command error\n");
 
-			TAILQ_REMOVE(&q->cmdqueue, cmd, next);
-			if (cmd->flags & KBC_CMDFLAG_SYNC)
+			if (cmd->flags & KBC_CMDFLAG_SYNC) {
 				wakeup(cmd);
-			else {
+				cmd = TAILQ_NEXT(cmd, next);
+			} else {
+				TAILQ_REMOVE(&q->cmdqueue, cmd, next);
 				timeout_del(&t->t_cleanup);
 				TAILQ_INSERT_TAIL(&q->freequeue, cmd, next);
+				cmd = TAILQ_FIRST(&q->cmdqueue);
 			}
-			cmd = TAILQ_FIRST(&q->cmdqueue);
 		} while (cmd);
 		return;
 	}
@@ -863,14 +864,16 @@ pckbc_cmdresponse(t, slot, data)
 		return (0);
 
 	/* dequeue: */
-	TAILQ_REMOVE(&q->cmdqueue, cmd, next);
-	if (cmd->flags & KBC_CMDFLAG_SYNC)
+	if (cmd->flags & KBC_CMDFLAG_SYNC) {
 		wakeup(cmd);
-	else {
+		cmd = TAILQ_NEXT(cmd, next);
+	} else {
+		TAILQ_REMOVE(&q->cmdqueue, cmd, next);
 		timeout_del(&t->t_cleanup);
 		TAILQ_INSERT_TAIL(&q->freequeue, cmd, next);
+		cmd = TAILQ_FIRST(&q->cmdqueue);
 	}
-	if (!CMD_IN_QUEUE(q))
+	if (cmd == NULL)
 		return (1);
 restart:
 	pckbc_start(t, slot);
@@ -932,8 +935,10 @@ pckbc_enqueue_cmd(self, slot, cmd, len, responselen, sync, respbuf)
 		if ((res = tsleep(nc, 0, "kbccmd", 1*hz))) {
 			TAILQ_REMOVE(&q->cmdqueue, nc, next);
 			pckbc_cleanup(t);
-		} else
+		} else {
+			TAILQ_REMOVE(&q->cmdqueue, nc, next);
 			res = nc->status;
+		}
 	} else
 		timeout_add_sec(&t->t_cleanup, 1);