USB keyboard lockups & patch to (probably) fix.

[Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx>]

    Twice in the last month my workstation has locked up.  It was very odd.
    Everything would be going just fine, a ton of open windows, browser, etc,
    but in both cases I simply hit a key on the keyboard, and poof, the box
    decided to take a permanent siesta.

    I was able to get a dump the second time.  It turns out that the CLIST
    module's freelist (kern/tty_subr.c primarily) was getting corrupted.

    And then it hit me... tty's aren't the only things that use the clist
    module.  The keyboard code does too, and I had recently switched to 
    a USB keyboard.  It turns out that the clist code was only protecting
    itself against tty interrupts (spltty()), so every time you hit a key
    on a USB keyboard while other clist-using entities (like anything that
    uses a tty or pty) are active, there is a small but not-zero chance that
    you will bork the machine.

    So, here's a tentitive patch to fix the problem.  Basically it protects
    clist operations with a critical section instead of with spltty().  I
    also revamped the clist code a bit and added a large number of KKASSERTs
    to make it more robust.

    My intention is to commit this on Friday and to also slip the 
    DragonFly_Stable tag for the commit.  Testing would be appreciated!

					-Matt
					Matthew Dillon 
					<dillon@xxxxxxxxxxxxx>

Index: dev/misc/kbd/kbd.c
===================================================================
RCS file: /cvs/src/sys/dev/misc/kbd/kbd.c,v
retrieving revision 1.12
diff -u -r1.12 kbd.c
--- dev/misc/kbd/kbd.c	19 Sep 2004 02:15:44 -0000	1.12
+++ dev/misc/kbd/kbd.c	5 Oct 2004 20:54:26 -0000
@@ -26,6 +26,13 @@
  * $FreeBSD: src/sys/dev/kbd/kbd.c,v 1.17.2.2 2001/07/30 16:46:43 yokota Exp $
  * $DragonFly: src/sys/dev/misc/kbd/kbd.c,v 1.12 2004/09/19 02:15:44 dillon Exp $
  */
+/*
+ * Generic keyboard driver.
+ *
+ * Interrupt note: keyboards use clist functions and since usb keyboard
+ * interrupts are not protected by spltty(), we must use a critical section
+ * to protect against corruption.
+ */
 
 #include "opt_kbd.h"
 
@@ -39,6 +46,8 @@
 #include <sys/poll.h>
 #include <sys/vnode.h>
 #include <sys/uio.h>
+#include <sys/thread.h>
+#include <sys/thread2.h>
 
 #include <machine/console.h>
 
@@ -80,9 +89,7 @@
 	keyboard_t **new_kbd;
 	keyboard_switch_t **new_kbdsw;
 	int newsize;
-	int s;
 
-	s = spltty();
 	newsize = ((keyboards + ARRAY_DELTA)/ARRAY_DELTA)*ARRAY_DELTA;
 	new_kbd = malloc(sizeof(*new_kbd) * newsize, M_DEVBUF,
 				M_WAITOK | M_ZERO);
@@ -90,6 +97,7 @@
 				M_WAITOK | M_ZERO);
 	bcopy(keyboard, new_kbd, sizeof(*keyboard)*keyboards);
 	bcopy(kbdsw, new_kbdsw, sizeof(*kbdsw)*keyboards);
+	crit_enter();
 	if (keyboards > 1) {
 		free(keyboard, M_DEVBUF);
 		free(kbdsw, M_DEVBUF);
@@ -97,7 +105,7 @@
 	keyboard = new_kbd;
 	kbdsw = new_kbdsw;
 	keyboards = newsize;
-	splx(s);
+	crit_exit();
 
 	if (bootverbose)
 		printf("kbd: new array size %d\n", keyboards);
@@ -226,23 +234,22 @@
 kbd_unregister(keyboard_t *kbd)
 {
 	int error;
-	int s;
 
 	if ((kbd->kb_index < 0) || (kbd->kb_index >= keyboards))
 		return ENOENT;
 	if (keyboard[kbd->kb_index] != kbd)
 		return ENOENT;
 
-	s = spltty();
+	crit_enter();
 	if (KBD_IS_BUSY(kbd)) {
 		error = (*kbd->kb_callback.kc_func)(kbd, KBDIO_UNLOADING,
 						    kbd->kb_callback.kc_arg);
 		if (error) {
-			splx(s);
+			crit_exit();
 			return error;
 		}
 		if (KBD_IS_BUSY(kbd)) {
-			splx(s);
+			crit_exit();
 			return EBUSY;
 		}
 	}
@@ -250,7 +257,7 @@
 	keyboard[kbd->kb_index] = NULL;
 	kbdsw[kbd->kb_index] = NULL;
 
-	splx(s);
+	crit_exit();
 	return 0;
 }
 
@@ -315,16 +322,15 @@
 	     void *arg)
 {
 	int index;
-	int s;
 
 	if (func == NULL)
 		return -1;
 
-	s = spltty();
+	crit_enter();
 	index = kbd_find_keyboard(driver, unit);
 	if (index >= 0) {
 		if (KBD_IS_BUSY(keyboard[index])) {
-			splx(s);
+			crit_exit();
 			return -1;
 		}
 		callout_init(&keyboard[index]->kb_atkbd_timeout_ch);
@@ -334,7 +340,7 @@
 		keyboard[index]->kb_callback.kc_arg = arg;
 		(*kbdsw[index]->clear_state)(keyboard[index]);
 	}
-	splx(s);
+	crit_exit();
 	return index;
 }
 
@@ -342,9 +348,8 @@
 kbd_release(keyboard_t *kbd, void *id)
 {
 	int error;
-	int s;
 
-	s = spltty();
+	crit_enter();
 	if (!KBD_IS_VALID(kbd) || !KBD_IS_BUSY(kbd)) {
 		error = EINVAL;
 	} else if (kbd->kb_token != id) {
@@ -358,7 +363,7 @@
 		(*kbdsw[kbd->kb_index]->clear_state)(kbd);
 		error = 0;
 	}
-	splx(s);
+	crit_exit();
 	return error;
 }
 
@@ -367,9 +372,8 @@
 		    void *arg)
 {
 	int error;
-	int s;
 
-	s = spltty();
+	crit_enter();
 	if (!KBD_IS_VALID(kbd) || !KBD_IS_BUSY(kbd)) {
 		error = EINVAL;
 	} else if (kbd->kb_token != id) {
@@ -381,7 +385,7 @@
 		kbd->kb_callback.kc_arg = arg;
 		error = 0;
 	}
-	splx(s);
+	crit_exit();
 	return error;
 }
 
@@ -523,20 +527,19 @@
 {
 	keyboard_t *kbd;
 	genkbd_softc_t *sc;
-	int s;
 	int i;
 
-	s = spltty();
+	crit_enter();
 	sc = dev->si_drv1;
 	kbd = kbd_get_keyboard(KBD_INDEX(dev));
 	if ((sc == NULL) || (kbd == NULL) || !KBD_IS_VALID(kbd)) {
-		splx(s);
+		crit_exit();
 		return ENXIO;
 	}
 	i = kbd_allocate(kbd->kb_name, kbd->kb_unit, sc,
 			 genkbd_event, (void *)sc);
 	if (i < 0) {
-		splx(s);
+		crit_exit();
 		return EBUSY;
 	}
 	/* assert(i == kbd->kb_index) */
@@ -553,7 +556,7 @@
 	clist_alloc_cblocks(&sc->gkb_q, KB_QSIZE, KB_QSIZE/2); /* XXX */
 	sc->gkb_rsel.si_flags = 0;
 	sc->gkb_rsel.si_pid = 0;
-	splx(s);
+	crit_exit();
 
 	return 0;
 }
@@ -563,13 +566,12 @@
 {
 	keyboard_t *kbd;
 	genkbd_softc_t *sc;
-	int s;
 
 	/*
 	 * NOTE: the device may have already become invalid.
 	 * kbd == NULL || !KBD_IS_VALID(kbd)
 	 */
-	s = spltty();
+	crit_enter();
 	sc = dev->si_drv1;
 	kbd = kbd_get_keyboard(KBD_INDEX(dev));
 	if ((sc == NULL) || (kbd == NULL) || !KBD_IS_VALID(kbd)) {
@@ -580,7 +582,7 @@
 		clist_free_cblocks(&sc->gkb_q);
 #endif
 	}
-	splx(s);
+	crit_exit();
 	return 0;
 }
 
@@ -592,35 +594,34 @@
 	u_char buffer[KB_BUFSIZE];
 	int len;
 	int error;
-	int s;
 
 	/* wait for input */
-	s = spltty();
+	crit_enter();
 	sc = dev->si_drv1;
 	kbd = kbd_get_keyboard(KBD_INDEX(dev));
 	if ((sc == NULL) || (kbd == NULL) || !KBD_IS_VALID(kbd)) {
-		splx(s);
+		crit_exit();
 		return ENXIO;
 	}
 	while (sc->gkb_q.c_cc == 0) {
 		if (flag & IO_NDELAY) {
-			splx(s);
+			crit_exit();
 			return EWOULDBLOCK;
 		}
 		sc->gkb_flags |= KB_ASLEEP;
 		error = tsleep((caddr_t)sc, PCATCH, "kbdrea", 0);
 		kbd = kbd_get_keyboard(KBD_INDEX(dev));
 		if ((kbd == NULL) || !KBD_IS_VALID(kbd)) {
-			splx(s);
+			crit_exit();
 			return ENXIO;	/* our keyboard has gone... */
 		}
 		if (error) {
 			sc->gkb_flags &= ~KB_ASLEEP;
-			splx(s);
+			crit_exit();
 			return error;
 		}
 	}
-	splx(s);
+	crit_exit();
 
 	/* copy as much input as possible */
 	error = 0;
@@ -669,10 +670,9 @@
 	keyboard_t *kbd;
 	genkbd_softc_t *sc;
 	int revents;
-	int s;
 
 	revents = 0;
-	s = spltty();
+	crit_enter();
 	sc = dev->si_drv1;
 	kbd = kbd_get_keyboard(KBD_INDEX(dev));
 	if ((sc == NULL) || (kbd == NULL) || !KBD_IS_VALID(kbd)) {
@@ -683,7 +683,7 @@
 		else
 			selrecord(td, &sc->gkb_rsel);
 	}
-	splx(s);
+	crit_exit();
 	return revents;
 }
 
@@ -799,10 +799,9 @@
 {
 	keyarg_t *keyp;
 	fkeyarg_t *fkeyp;
-	int s;
 	int i;
 
-	s = spltty();
+	crit_enter();
 	switch (cmd) {
 
 	case KDGKBINFO:		/* get keyboard information */
@@ -834,7 +833,7 @@
 		bcopy(arg, kbd->kb_keymap, sizeof(*kbd->kb_keymap));
 		break;
 #else
-		splx(s);
+		crit_exit();
 		return ENODEV;
 #endif
 
@@ -842,7 +841,7 @@
 		keyp = (keyarg_t *)arg;
 		if (keyp->keynum >= sizeof(kbd->kb_keymap->key)
 					/sizeof(kbd->kb_keymap->key[0])) {
-			splx(s);
+			crit_exit();
 			return EINVAL;
 		}
 		bcopy(&kbd->kb_keymap->key[keyp->keynum], &keyp->key,
@@ -853,14 +852,14 @@
 		keyp = (keyarg_t *)arg;
 		if (keyp->keynum >= sizeof(kbd->kb_keymap->key)
 					/sizeof(kbd->kb_keymap->key[0])) {
-			splx(s);
+			crit_exit();
 			return EINVAL;
 		}
 		bcopy(&keyp->key, &kbd->kb_keymap->key[keyp->keynum],
 		      sizeof(keyp->key));
 		break;
 #else
-		splx(s);
+		crit_exit();
 		return ENODEV;
 #endif
 
@@ -872,14 +871,14 @@
 		bcopy(arg, kbd->kb_accentmap, sizeof(*kbd->kb_accentmap));
 		break;
 #else
-		splx(s);
+		crit_exit();
 		return ENODEV;
 #endif
 
 	case GETFKEY:		/* get functionkey string */
 		fkeyp = (fkeyarg_t *)arg;
 		if (fkeyp->keynum >= kbd->kb_fkeytab_size) {
-			splx(s);
+			crit_exit();
 			return EINVAL;
 		}
 		bcopy(kbd->kb_fkeytab[fkeyp->keynum].str, fkeyp->keydef,
@@ -890,7 +889,7 @@
 #ifndef KBD_DISABLE_KEYMAP_LOAD
 		fkeyp = (fkeyarg_t *)arg;
 		if (fkeyp->keynum >= kbd->kb_fkeytab_size) {
-			splx(s);
+			crit_exit();
 			return EINVAL;
 		}
 		kbd->kb_fkeytab[fkeyp->keynum].len = imin(fkeyp->flen, MAXFK);
@@ -898,16 +897,16 @@
 		      kbd->kb_fkeytab[fkeyp->keynum].len);
 		break;
 #else
-		splx(s);
+		crit_exit();
 		return ENODEV;
 #endif
 
 	default:
-		splx(s);
+		crit_exit();
 		return ENOIOCTL;
 	}
 
-	splx(s);
+	crit_exit();
 	return 0;
 }
 
Index: kern/tty.c
===================================================================
RCS file: /cvs/src/sys/kern/tty.c,v
retrieving revision 1.12
diff -u -r1.12 tty.c
--- kern/tty.c	13 Sep 2004 16:22:36 -0000	1.12
+++ kern/tty.c	5 Oct 2004 20:49:31 -0000
@@ -80,6 +80,7 @@
 #include <sys/proc.h>
 #define	TTYDEFCHARS
 #include <sys/tty.h>
+#include <sys/clist.h>
 #undef	TTYDEFCHARS
 #include <sys/fcntl.h>
 #include <sys/conf.h>
@@ -2554,8 +2555,7 @@
 
 	if (tp)
 		return(tp);
-        tp = malloc(sizeof *tp, M_TTYS, M_WAITOK);
-        bzero(tp, sizeof *tp);
+        tp = malloc(sizeof *tp, M_TTYS, M_WAITOK|M_ZERO);
 	ttyregister(tp);
         return (tp);
 }
Index: kern/tty_subr.c
===================================================================
RCS file: /cvs/src/sys/kern/tty_subr.c,v
retrieving revision 1.4
diff -u -r1.4 tty_subr.c
--- kern/tty_subr.c	8 Jan 2004 18:39:18 -0000	1.4
+++ kern/tty_subr.c	5 Oct 2004 20:55:22 -0000
@@ -30,6 +30,16 @@
 
 /*
  * clist support routines
+ *
+ * NOTE on cblock->c_cf:	This pointer may point at the base of a cblock,
+ *				which is &cblock->c_info[0], but will never
+ *				point at the end of a cblock (char *)(cblk + 1)
+ *				
+ * NOTE on cblock->c_cl:	This pointer will never point at the base of
+ *				a block but may point at the end of one.
+ *
+ * These routines may be used by more then just ttys, so a critical section
+ * must be used to access the free list, and for general safety.
  */
 
 #include <sys/param.h>
@@ -38,6 +48,7 @@
 #include <sys/malloc.h>
 #include <sys/tty.h>
 #include <sys/clist.h>
+#include <sys/thread2.h>
 
 static void clist_init (void *);
 SYSINIT(clist, SI_SUB_CLIST, SI_ORDER_FIRST, clist_init, NULL)
@@ -76,8 +87,7 @@
  */
 /* ARGSUSED*/
 static void
-clist_init(dummy)
-	void *dummy;
+clist_init(void *dummy)
 {
 	/*
 	 * Allocate an initial base set of cblocks as a 'slush'.
@@ -88,46 +98,55 @@
 	 * interrupt handlers when it may be unsafe to call malloc().
 	 */
 	cblock_alloc_cblocks(cslushcount = INITIAL_CBLOCKS);
+	KKASSERT(sizeof(struct cblock) == CBLOCK);
 }
 
 /*
  * Remove a cblock from the cfreelist queue and return a pointer
  * to it.
+ *
+ * May not block.
  */
-static __inline struct cblock *
-cblock_alloc()
+static struct cblock *
+cblock_alloc(void)
 {
 	struct cblock *cblockp;
 
 	cblockp = cfreelist;
 	if (cblockp == NULL)
 		panic("clist reservation botch");
-	cfreelist = cblockp->c_next;
-	cblockp->c_next = NULL;
+	KKASSERT(cblockp->c_head.ch_magic == CLIST_MAGIC_FREE);
+	cfreelist = cblockp->c_head.ch_next;
+	cblockp->c_head.ch_next = NULL;
+	cblockp->c_head.ch_magic = CLIST_MAGIC_USED;
 	cfreecount -= CBSIZE;
 	return (cblockp);
 }
 
 /*
  * Add a cblock to the cfreelist queue.
+ *
+ * May not block, must be called in a critical section
  */
-static __inline void
-cblock_free(cblockp)
-	struct cblock *cblockp;
+static void
+cblock_free(struct cblock *cblockp)
 {
 	if (isset(cblockp->c_quote, CBQSIZE * NBBY - 1))
 		bzero(cblockp->c_quote, sizeof cblockp->c_quote);
-	cblockp->c_next = cfreelist;
+	KKASSERT(cblockp->c_head.ch_magic == CLIST_MAGIC_USED);
+	cblockp->c_head.ch_next = cfreelist;
+	cblockp->c_head.ch_magic = CLIST_MAGIC_FREE;
 	cfreelist = cblockp;
 	cfreecount += CBSIZE;
 }
 
 /*
  * Allocate some cblocks for the cfreelist queue.
+ *
+ * This routine my  block, but still must be called in a critical section
  */
 static void
-cblock_alloc_cblocks(number)
-	int number;
+cblock_alloc_cblocks(int number)
 {
 	int i;
 	struct cblock *cbp;
@@ -139,25 +158,23 @@
 "clist_alloc_cblocks: M_NOWAIT malloc failed, trying M_WAITOK\n");
 			cbp = malloc(sizeof *cbp, M_TTYS, M_WAITOK);
 		}
+		KKASSERT(((intptr_t)cbp & CROUND) == 0);
 		/*
 		 * Freed cblocks have zero quotes and garbage elsewhere.
 		 * Set the may-have-quote bit to force zeroing the quotes.
 		 */
 		setbit(cbp->c_quote, CBQSIZE * NBBY - 1);
+		cbp->c_head.ch_magic = CLIST_MAGIC_USED;
 		cblock_free(cbp);
 	}
 	ctotcount += number;
 }
 
 /*
- * Set the cblock allocation policy for a a clist.
- * Must be called in process context at spltty().
+ * Set the cblock allocation policy for a clist.
  */
 void
-clist_alloc_cblocks(clistp, ccmax, ccreserved)
-	struct clist *clistp;
-	int ccmax;
-	int ccreserved;
+clist_alloc_cblocks(struct clist *clistp, int ccmax, int ccreserved)
 {
 	int dcbr;
 
@@ -169,25 +186,31 @@
 	if (ccreserved != 0)
 		ccreserved += CBSIZE - 1;
 
+	crit_enter();
 	clistp->c_cbmax = roundup(ccmax, CBSIZE) / CBSIZE;
 	dcbr = roundup(ccreserved, CBSIZE) / CBSIZE - clistp->c_cbreserved;
-	if (dcbr >= 0)
-		cblock_alloc_cblocks(dcbr);
-	else {
+	if (dcbr >= 0) {
+		clistp->c_cbreserved += dcbr;	/* atomic w/c_cbmax */
+		cblock_alloc_cblocks(dcbr);	/* may block */
+	} else {
+		KKASSERT(clistp->c_cbcount <= clistp->c_cbreserved);
 		if (clistp->c_cbreserved + dcbr < clistp->c_cbcount)
 			dcbr = clistp->c_cbcount - clistp->c_cbreserved;
-		cblock_free_cblocks(-dcbr);
+		clistp->c_cbreserved += dcbr;	/* atomic w/c_cbmax */
+		cblock_free_cblocks(-dcbr);	/* may block */
 	}
-	clistp->c_cbreserved += dcbr;
+	KKASSERT(clistp->c_cbreserved >= 0);
+	crit_exit();
 }
 
 /*
  * Free some cblocks from the cfreelist queue back to the
  * system malloc pool.
+ *
+ * Must be called from within a critical section.  May block.
  */
 static void
-cblock_free_cblocks(number)
-	int number;
+cblock_free_cblocks(int number)
 {
 	int i;
 
@@ -198,34 +221,34 @@
 
 /*
  * Free the cblocks reserved for a clist.
- * Must be called at spltty().
  */
 void
-clist_free_cblocks(clistp)
-	struct clist *clistp;
+clist_free_cblocks(struct clist *clistp)
 {
+	int cbreserved;
+
+	crit_enter();
 	if (clistp->c_cbcount != 0)
 		panic("freeing active clist cblocks");
-	cblock_free_cblocks(clistp->c_cbreserved);
+	cbreserved = clistp->c_cbreserved;
 	clistp->c_cbmax = 0;
 	clistp->c_cbreserved = 0;
+	cblock_free_cblocks(cbreserved); /* may block */
+	crit_exit();
 }
 
 /*
  * Get a character from the head of a clist.
  */
 int
-getc(clistp)
-	struct clist *clistp;
+getc(struct clist *clistp)
 {
 	int chr = -1;
-	int s;
 	struct cblock *cblockp;
 
-	s = spltty();
-
-	/* If there are characters in the list, get one */
+	crit_enter();
 	if (clistp->c_cc) {
+		KKASSERT(((intptr_t)clistp->c_cf & CROUND) != 0);
 		cblockp = (struct cblock *)((intptr_t)clistp->c_cf & ~CROUND);
 		chr = (u_char)*clistp->c_cf;
 
@@ -247,9 +270,11 @@
 		 * last pointers to NULL. In either case, free the
 		 * current cblock.
 		 */
-		if ((clistp->c_cf >= (char *)(cblockp+1)) || (clistp->c_cc == 0)) {
+		KKASSERT(clistp->c_cf <= (char *)(cblockp + 1));
+		if ((clistp->c_cf == (char *)(cblockp + 1)) ||
+		    (clistp->c_cc == 0)) {
 			if (clistp->c_cc > 0) {
-				clistp->c_cf = cblockp->c_next->c_info;
+				clistp->c_cf = cblockp->c_head.ch_next->c_info;
 			} else {
 				clistp->c_cf = clistp->c_cl = NULL;
 			}
@@ -258,8 +283,7 @@
 				++cslushcount;
 		}
 	}
-
-	splx(s);
+	crit_exit();
 	return (chr);
 }
 
@@ -269,20 +293,16 @@
  * actually copied.
  */
 int
-q_to_b(clistp, dest, amount)
-	struct clist *clistp;
-	char *dest;
-	int amount;
+q_to_b(struct clist *clistp, char *dest, int amount)
 {
 	struct cblock *cblockp;
 	struct cblock *cblockn;
 	char *dest_orig = dest;
 	int numc;
-	int s;
-
-	s = spltty();
 
+	crit_enter();
 	while (clistp && amount && (clistp->c_cc > 0)) {
+		KKASSERT(((intptr_t)clistp->c_cf & CROUND) != 0);
 		cblockp = (struct cblock *)((intptr_t)clistp->c_cf & ~CROUND);
 		cblockn = cblockp + 1; /* pointer arithmetic! */
 		numc = min(amount, (char *)cblockn - clistp->c_cf);
@@ -298,9 +318,11 @@
 		 * and last pointer to NULL. In either case, free the
 		 * current cblock.
 		 */
-		if ((clistp->c_cf >= (char *)cblockn) || (clistp->c_cc == 0)) {
+		KKASSERT(clistp->c_cf <= (char *)cblockn);
+		if ((clistp->c_cf == (char *)cblockn) || (clistp->c_cc == 0)) {
 			if (clistp->c_cc > 0) {
-				clistp->c_cf = cblockp->c_next->c_info;
+				KKASSERT(cblockp->c_head.ch_next != NULL);
+				clistp->c_cf = cblockp->c_head.ch_next->c_info;
 			} else {
 				clistp->c_cf = clistp->c_cl = NULL;
 			}
@@ -309,8 +331,7 @@
 				++cslushcount;
 		}
 	}
-
-	splx(s);
+	crit_exit();
 	return (dest - dest_orig);
 }
 
@@ -318,18 +339,15 @@
  * Flush 'amount' of chars, beginning at head of clist 'clistp'.
  */
 void
-ndflush(clistp, amount)
-	struct clist *clistp;
-	int amount;
+ndflush(struct clist *clistp, int amount)
 {
 	struct cblock *cblockp;
 	struct cblock *cblockn;
 	int numc;
-	int s;
-
-	s = spltty();
 
+	crit_enter();
 	while (amount && (clistp->c_cc > 0)) {
+		KKASSERT(((intptr_t)clistp->c_cf & CROUND) != 0);
 		cblockp = (struct cblock *)((intptr_t)clistp->c_cf & ~CROUND);
 		cblockn = cblockp + 1; /* pointer arithmetic! */
 		numc = min(amount, (char *)cblockn - clistp->c_cf);
@@ -343,9 +361,11 @@
 		 * and last pointer to NULL. In either case, free the
 		 * current cblock.
 		 */
-		if ((clistp->c_cf >= (char *)cblockn) || (clistp->c_cc == 0)) {
+		KKASSERT(clistp->c_cf <= (char *)cblockn);
+		if (clistp->c_cf == (char *)cblockn || clistp->c_cc == 0) {
 			if (clistp->c_cc > 0) {
-				clistp->c_cf = cblockp->c_next->c_info;
+				KKASSERT(cblockp->c_head.ch_next != NULL);
+				clistp->c_cf = cblockp->c_head.ch_next->c_info;
 			} else {
 				clistp->c_cf = clistp->c_cl = NULL;
 			}
@@ -354,8 +374,7 @@
 				++cslushcount;
 		}
 	}
-
-	splx(s);
+	crit_exit();
 }
 
 /*
@@ -363,18 +382,20 @@
  * more clists, or 0 for success.
  */
 int
-putc(chr, clistp)
-	int chr;
-	struct clist *clistp;
+putc(int chr, struct clist *clistp)
 {
 	struct cblock *cblockp;
-	int s;
 
-	s = spltty();
+	crit_enter();
 
+	/*
+	 * Note: this section may point c_cl at the base of a cblock.  This
+	 * is a temporary violation of the requirements for c_cl, we
+	 * increment it before returning.
+	 */
 	if (clistp->c_cl == NULL) {
 		if (clistp->c_cbreserved < 1) {
-			splx(s);
+			crit_exit();
 			printf("putc to a clist with no reserved cblocks\n");
 			return (-1);		/* nothing done */
 		}
@@ -390,14 +411,14 @@
 			if (clistp->c_cbcount >= clistp->c_cbreserved) {
 				if (clistp->c_cbcount >= clistp->c_cbmax
 				    || cslushcount <= 0) {
-					splx(s);
+					crit_exit();
 					return (-1);
 				}
 				--cslushcount;
 			}
 			cblockp = cblock_alloc();
 			clistp->c_cbcount++;
-			prev->c_next = cblockp;
+			prev->c_head.ch_next = cblockp;
 			clistp->c_cl = cblockp->c_info;
 		}
 	}
@@ -412,13 +433,14 @@
 		 * may be quoted.
 		 */
 		setbit(cblockp->c_quote, CBQSIZE * NBBY - 1);
-	} else
+	} else {
 		clrbit(cblockp->c_quote, clistp->c_cl - (char *)cblockp->c_info);
+	}
 
 	*clistp->c_cl++ = chr;
 	clistp->c_cc++;
 
-	splx(s);
+	crit_exit();
 	return (0);
 }
 
@@ -427,16 +449,12 @@
  * number of characters not copied.
  */
 int
-b_to_q(src, amount, clistp)
-	char *src;
-	int amount;
-	struct clist *clistp;
+b_to_q(char *src, int amount, struct clist *clistp)
 {
 	struct cblock *cblockp;
 	char *firstbyte, *lastbyte;
 	u_char startmask, endmask;
 	int startbit, endbit, num_between, numc;
-	int s;
 
 	/*
 	 * Avoid allocating an initial cblock and then not using it.
@@ -445,15 +463,16 @@
 	if (amount <= 0)
 		return (amount);
 
-	s = spltty();
+	crit_enter();
 
 	/*
-	 * If there are no cblocks assigned to this clist yet,
-	 * then get one.
+	 * Note: this section may point c_cl at the base of a cblock.  This
+	 * is a temporary violation of the requirements for c_cl.  Since
+	 * amount is non-zero we will not return with it in that state.
 	 */
 	if (clistp->c_cl == NULL) {
 		if (clistp->c_cbreserved < 1) {
-			splx(s);
+			crit_exit();
 			printf("b_to_q to a clist with no reserved cblocks.\n");
 			return (amount);	/* nothing done */
 		}
@@ -462,6 +481,10 @@
 		clistp->c_cf = clistp->c_cl = cblockp->c_info;
 		clistp->c_cc = 0;
 	} else {
+		/*
+		 * c_cl may legally point past the end of the block, which
+		 * falls through to the 'get another cblock' code below.
+		 */
 		cblockp = (struct cblock *)((intptr_t)clistp->c_cl & ~CROUND);
 	}
 
@@ -475,14 +498,14 @@
 			if (clistp->c_cbcount >= clistp->c_cbreserved) {
 				if (clistp->c_cbcount >= clistp->c_cbmax
 				    || cslushcount <= 0) {
-					splx(s);
+					crit_exit();
 					return (amount);
 				}
 				--cslushcount;
 			}
 			cblockp = cblock_alloc();
 			clistp->c_cbcount++;
-			prev->c_next = cblockp;
+			prev->c_head.ch_next = cblockp;
 			clistp->c_cl = cblockp->c_info;
 		}
 
@@ -540,11 +563,9 @@
 		 * cblock) we prepare for the assignment of 'prev'
 		 * above.
 		 */
-		cblockp += 1;
-
+		++cblockp;
 	}
-
-	splx(s);
+	crit_exit();
 	return (amount);
 }
 
@@ -552,12 +573,12 @@
  * Get the next character in the clist. Store it at dst. Don't
  * advance any clist pointers, but return a pointer to the next
  * character position.
+ *
+ * Must be called at spltty().  This routine may not run in a critical
+ * section and so may not call the cblock allocator/deallocator.
  */
 char *
-nextc(clistp, cp, dst)
-	struct clist *clistp;
-	char *cp;
-	int *dst;
+nextc(struct clist *clistp, char *cp, int *dst)
 {
 	struct cblock *cblockp;
 
@@ -572,7 +593,7 @@
 		 * cblock, advance to the next cblock.
 		 */
 		if (((intptr_t)cp & CROUND) == 0)
-			cp = ((struct cblock *)cp - 1)->c_next->c_info;
+			cp = ((struct cblock *)cp - 1)->c_head.ch_next->c_info;
 		cblockp = (struct cblock *)((intptr_t)cp & ~CROUND);
 
 		/*
@@ -591,17 +612,20 @@
  * "Unput" a character from a clist.
  */
 int
-unputc(clistp)
-	struct clist *clistp;
+unputc(struct clist *clistp)
 {
 	struct cblock *cblockp = 0, *cbp = 0;
-	int s;
 	int chr = -1;
 
-
-	s = spltty();
+	crit_enter();
 
 	if (clistp->c_cc) {
+		/*
+		 * note that clistp->c_cl will never point at the base
+		 * of a cblock (cblock->c_info) (see assert this later on),
+		 * but it may point past the end of one.  We temporarily
+		 * violate this in the decrement below but then we fix it up.
+		 */
 		--clistp->c_cc;
 		--clistp->c_cl;
 
@@ -619,30 +643,39 @@
 		 * If all of the characters have been unput in this
 		 * one.
+		 *
+		 * if c_cc is 0 clistp->c_cl may end up pointing at
+		 * cblockp->c_info, which is illegal, but the case will be 
+		 * taken care of near the end of the routine.  Otherwise
+		 * there *MUST* be another cblock, find it.
 		 */
-		if (clistp->c_cc && (clistp->c_cl <= (char *)cblockp->c_info)) {
+		KKASSERT(clistp->c_cl >= (char *)cblockp->c_info);
+		if (clistp->c_cc && (clistp->c_cl == (char *)cblockp->c_info)) {
 			cbp = (struct cblock *)((intptr_t)clistp->c_cf & ~CROUND);
 
-			while (cbp->c_next != cblockp)
-				cbp = cbp->c_next;
+			while (cbp->c_head.ch_next != cblockp)
+				cbp = cbp->c_head.ch_next;
+			cbp->c_head.ch_next = NULL;
 
 			/*
 			 * When the previous cblock is at the end, the 'last'
 			 * pointer always points (invalidly) one past.
 			 */
-			clistp->c_cl = (char *)(cbp+1);
+			clistp->c_cl = (char *)(cbp + 1);
 			cblock_free(cblockp);
 			if (--clistp->c_cbcount >= clistp->c_cbreserved)
 				++cslushcount;
-			cbp->c_next = NULL;
 		}
 	}
 
 	/*
 	 * If there are no more characters on the list, then
-	 * free the last cblock.
+	 * free the last cblock.   It should not be possible for c->cl
+	 * to be pointing past the end of a block due to our decrement
+	 * of it way above.
 	 */
-	if ((clistp->c_cc == 0) && clistp->c_cl) {
+	if (clistp->c_cc == 0 && clistp->c_cl) {
+		KKASSERT(((intptr_t)clistp->c_cl & CROUND) != 0);
 		cblockp = (struct cblock *)((intptr_t)clistp->c_cl & ~CROUND);
 		cblock_free(cblockp);
 		if (--clistp->c_cbcount >= clistp->c_cbreserved)
@@ -650,7 +683,7 @@
 		clistp->c_cf = clistp->c_cl = NULL;
 	}
 
-	splx(s);
+	crit_exit();
 	return (chr);
 }
 
@@ -659,12 +692,11 @@
  * preserving quote bits.
  */
 void
-catq(src_clistp, dest_clistp)
-	struct clist *src_clistp, *dest_clistp;
+catq(struct clist *src_clistp, struct clist *dest_clistp)
 {
-	int chr, s;
+	int chr;
 
-	s = spltty();
+	crit_enter();
 	/*
 	 * If the destination clist is empty (has no cblocks atttached),
 	 * and there are no possible complications with the resource counters,
@@ -682,11 +714,10 @@
 		dest_clistp->c_cbcount = src_clistp->c_cbcount;
 		src_clistp->c_cbcount = 0;
 
-		splx(s);
+		crit_exit();
 		return;
 	}
-
-	splx(s);
+	crit_exit();
 
 	/*
 	 * XXX  This should probably be optimized to more than one
Index: sys/clist.h
===================================================================
RCS file: /cvs/src/sys/sys/clist.h,v
retrieving revision 1.2
diff -u -r1.2 clist.h
--- sys/clist.h	17 Jun 2003 04:28:58 -0000	1.2
+++ sys/clist.h	5 Oct 2004 20:32:08 -0000
@@ -38,8 +38,22 @@
 #ifndef _SYS_CLIST_H_
 #define _SYS_CLIST_H_
 
+#define CBLOCK	128		/* Clist block size, must be a power of 2. */
+#define CBQSIZE	(CBLOCK/NBBY)	/* Quote bytes/cblock - can do better. */
+				/* Data chars/clist. */
+#define CBSIZE	(CBLOCK - sizeof(struct cblockhead) - CBQSIZE)
+#define CROUND	(CBLOCK - 1)	/* Clist rounding. */
+
+struct cblockhead {
+	struct cblock *ch_next;
+	int	ch_magic;
+};
+
+#define CLIST_MAGIC_FREE	0x434c0102
+#define CLIST_MAGIC_USED	0x434c8182
+
 struct cblock {
-	struct cblock *c_next;			/* next cblock in queue */
+	struct cblockhead c_head;		/* header */
 	unsigned char c_quote[CBQSIZE];		/* quoted characters */
 	unsigned char c_info[CBSIZE];		/* characters */
 };
Index: sys/param.h
===================================================================
RCS file: /cvs/src/sys/sys/param.h,v
retrieving revision 1.17
diff -u -r1.17 param.h
--- sys/param.h	23 Aug 2004 16:03:44 -0000	1.17
+++ sys/param.h	5 Oct 2004 21:33:54 -0000
@@ -146,21 +146,6 @@
 #endif
 
 /*
- * Clustering of hardware pages on machines with ridiculously small
- * page sizes is done here.  The paging subsystem deals with units of
- * CLSIZE pte's describing PAGE_SIZE (from machine/machparam.h) pages each.
- */
-#if 0
-#define CLBYTES		(CLSIZE*PAGE_SIZE)
-#endif
-
-#define CBLOCK	128		/* Clist block size, must be a power of 2. */
-#define CBQSIZE	(CBLOCK/NBBY)	/* Quote bytes/cblock - can do better. */
-				/* Data chars/clist. */
-#define CBSIZE	(CBLOCK - sizeof(struct cblock *) - CBQSIZE)
-#define CROUND	(CBLOCK - 1)	/* Clist rounding. */
-
-/*
  * File system parameters and macros.
  *
  * MAXBSIZE -	Filesystems are made out of blocks of at most MAXBSIZE bytes