From a38e57d5b87a200ae94ece676d5326fb9eca15fb Mon Sep 17 00:00:00 2001 From: Matthew Dillon Date: Thu, 7 Oct 2004 01:32:04 +0000 Subject: [PATCH] Fix a bug in the tty clist code. The clist code was only protecting itself with spltty() but the keyboard code uses clists too, and USB interrupts (for USB keyboards) aren't protected by spltty(). Generally convert the spl's to critical sections to solve the problem. --- sys/dev/misc/kbd/kbd.c | 97 ++++++++-------- sys/kern/tty.c | 6 +- sys/kern/tty_subr.c | 253 +++++++++++++++++++++++------------------ 3 files changed, 193 insertions(+), 163 deletions(-) diff --git a/sys/dev/misc/kbd/kbd.c b/sys/dev/misc/kbd/kbd.c index 3b937c99d4..afa79ad215 100644 --- a/sys/dev/misc/kbd/kbd.c +++ b/sys/dev/misc/kbd/kbd.c @@ -24,7 +24,14 @@ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. * * $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 $ + * $DragonFly: src/sys/dev/misc/kbd/kbd.c,v 1.13 2004/10/07 01:32:04 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 #include #include +#include +#include #include @@ -80,9 +89,7 @@ kbd_realloc_array(void) 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 @@ kbd_realloc_array(void) 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 @@ kbd_realloc_array(void) 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 @@ int 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 @@ kbd_unregister(keyboard_t *kbd) keyboard[kbd->kb_index] = NULL; kbdsw[kbd->kb_index] = NULL; - splx(s); + crit_exit(); return 0; } @@ -315,16 +322,15 @@ kbd_allocate(char *driver, int unit, void *id, kbd_callback_func_t *func, 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 @@ kbd_allocate(char *driver, int unit, void *id, kbd_callback_func_t *func, keyboard[index]->kb_callback.kc_arg = arg; (*kbdsw[index]->clear_state)(keyboard[index]); } - splx(s); + crit_exit(); return index; } @@ -342,9 +348,8 @@ int 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 @@ kbd_release(keyboard_t *kbd, void *id) (*kbdsw[kbd->kb_index]->clear_state)(kbd); error = 0; } - splx(s); + crit_exit(); return error; } @@ -367,9 +372,8 @@ kbd_change_callback(keyboard_t *kbd, void *id, kbd_callback_func_t *func, 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_change_callback(keyboard_t *kbd, void *id, kbd_callback_func_t *func, kbd->kb_callback.kc_arg = arg; error = 0; } - splx(s); + crit_exit(); return error; } @@ -523,20 +527,19 @@ genkbdopen(dev_t dev, int mode, int flag, d_thread_t *td) { 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 @@ genkbdopen(dev_t dev, int mode, int flag, d_thread_t *td) 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 @@ genkbdclose(dev_t dev, int mode, int flag, d_thread_t *td) { 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 @@ genkbdclose(dev_t dev, int mode, int flag, d_thread_t *td) clist_free_cblocks(&sc->gkb_q); #endif } - splx(s); + crit_exit(); return 0; } @@ -592,35 +594,34 @@ genkbdread(dev_t dev, struct uio *uio, int flag) 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 @@ genkbdpoll(dev_t dev, int events, d_thread_t *td) 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 @@ genkbdpoll(dev_t dev, int events, d_thread_t *td) else selrecord(td, &sc->gkb_rsel); } - splx(s); + crit_exit(); return revents; } @@ -799,10 +799,9 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) { 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 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) bcopy(arg, kbd->kb_keymap, sizeof(*kbd->kb_keymap)); break; #else - splx(s); + crit_exit(); return ENODEV; #endif @@ -842,7 +841,7 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) 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 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) 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 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) 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 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) #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 @@ genkbd_commonioctl(keyboard_t *kbd, u_long cmd, caddr_t arg) 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; } diff --git a/sys/kern/tty.c b/sys/kern/tty.c index 4659abe107..baeda44811 100644 --- a/sys/kern/tty.c +++ b/sys/kern/tty.c @@ -37,7 +37,7 @@ * * @(#)tty.c 8.8 (Berkeley) 1/21/94 * $FreeBSD: src/sys/kern/tty.c,v 1.129.2.5 2002/03/11 01:32:31 dd Exp $ - * $DragonFly: src/sys/kern/tty.c,v 1.12 2004/09/13 16:22:36 dillon Exp $ + * $DragonFly: src/sys/kern/tty.c,v 1.13 2004/10/07 01:32:03 dillon Exp $ */ /*- @@ -80,6 +80,7 @@ #include #define TTYDEFCHARS #include +#include #undef TTYDEFCHARS #include #include @@ -2554,8 +2555,7 @@ ttymalloc(tp) 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); } diff --git a/sys/kern/tty_subr.c b/sys/kern/tty_subr.c index 6dc00fed0a..ab99bc03b0 100644 --- a/sys/kern/tty_subr.c +++ b/sys/kern/tty_subr.c @@ -25,11 +25,21 @@ * SUCH DAMAGE. * * $FreeBSD: src/sys/kern/tty_subr.c,v 1.32 1999/08/28 00:46:21 peter Exp $ - * $DragonFly: src/sys/kern/tty_subr.c,v 1.4 2004/01/08 18:39:18 asmodai Exp $ + * $DragonFly: src/sys/kern/tty_subr.c,v 1.5 2004/10/07 01:32:03 dillon Exp $ */ /* * 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 @@ -38,6 +48,7 @@ #include #include #include +#include static void clist_init (void *); SYSINIT(clist, SI_SUB_CLIST, SI_ORDER_FIRST, clist_init, NULL) @@ -76,8 +87,7 @@ DB_SHOW_COMMAND(cbstat, cbstat) */ /* 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 @@ clist_init(dummy) * 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 @@ cblock_alloc_cblocks(number) "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 @@ clist_alloc_cblocks(clistp, ccmax, ccreserved) 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 @@ cblock_free_cblocks(number) /* * 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 @@ getc(clistp) * 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 @@ getc(clistp) ++cslushcount; } } - - splx(s); + crit_exit(); return (chr); } @@ -269,20 +293,16 @@ getc(clistp) * 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 @@ q_to_b(clistp, dest, amount) * 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 @@ q_to_b(clistp, dest, amount) ++cslushcount; } } - - splx(s); + crit_exit(); return (dest - dest_orig); } @@ -318,18 +339,15 @@ q_to_b(clistp, dest, amount) * 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 @@ ndflush(clistp, amount) * 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 @@ ndflush(clistp, amount) ++cslushcount; } } - - splx(s); + crit_exit(); } /* @@ -363,18 +382,20 @@ ndflush(clistp, amount) * 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 @@ putc(chr, clistp) 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 @@ putc(chr, clistp) * 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 @@ putc(chr, clistp) * 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 @@ b_to_q(src, amount, clistp) 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 @@ b_to_q(src, amount, clistp) 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 @@ b_to_q(src, amount, clistp) 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 @@ b_to_q(src, amount, clistp) * cblock) we prepare for the assignment of 'prev' * above. */ - cblockp += 1; - + ++cblockp; } - - splx(s); + crit_exit(); return (amount); } @@ -552,12 +573,12 @@ b_to_q(src, amount, clistp) * 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 @@ nextc(clistp, cp, dst) * 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 @@ nextc(clistp, cp, dst) * "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 @@ unputc(clistp) * If all of the characters have been unput in this * cblock, then find the previous one and free 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 @@ unputc(clistp) clistp->c_cf = clistp->c_cl = NULL; } - splx(s); + crit_exit(); return (chr); } @@ -659,12 +692,11 @@ unputc(clistp) * 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 @@ catq(src_clistp, dest_clistp) 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 -- 2.41.0