--- sys/bus/cam/cam_extend.c.orig +++ sys/bus/cam/cam_extend.c @@ -4,9 +4,9 @@ * * TRW Financial Systems, in accordance with their agreement with Carnegie * Mellon University, makes this software available to CMU to distribute - * or use in any manner that they see fit as long as this message is kept with - * the software. For this reason TFS also grants any other persons or - * organisations permission to use or modify this software. + * or use in any manner that they see fit as long as this message is kept + * with the software. For this reason TFS also grants any other persons + * or organisations permission to use or modify this software. * * TFS supplies this software to be publicly redistributed * on the understanding that TFS is not responsible for the correct @@ -27,6 +27,8 @@ #include #include #include +#include +#include #include "cam_extend.h" @@ -34,6 +36,7 @@ { int nelem; void **ps; + struct spinlock ex_lock; }; /* EXTEND_CHUNK: Number of extend slots to allocate whenever we need a new @@ -46,51 +49,99 @@ struct extend_array * cam_extend_new(void) { - return(kmalloc(sizeof(struct extend_array), M_DEVBUF, - M_INTWAIT | M_ZERO)); + struct extend_array *ea; + + ea = kmalloc(sizeof(struct extend_array), M_DEVBUF, + M_INTWAIT | M_ZERO); + spin_init(&ea->ex_lock, "cam_extend"); + return (ea); } +/* + * Returns the stored value on success, NULL on failure (bad index, + * allocation failure, or the slot already has storage). + * + * NOTE: The index is fully validated. The old code only checked the + * upper bound, so any negative index indexed before the array + * (an out-of-bounds kernel write) or faulted on a NULL ps. + * + * The array is also synchronized now: growth frees the old ps[] + * chunk, which racing lockless cam_extend_get() readers could + * still dereference (use-after-free read handed back as the + * stored object pointer). + */ void * cam_extend_set(struct extend_array *ea, int index, void *value) { + void **space, **oldps; + + if (index < 0 || index > INT_MAX - EXTEND_CHUNK) + return (NULL); + if (index >= ea->nelem) { - void **space; space = kmalloc(sizeof(void *) * (index + EXTEND_CHUNK), M_DEVBUF, M_INTWAIT | M_ZERO); + if (space == NULL) + return (NULL); - /* Make sure we have something to copy before we copy it */ - if (ea->nelem) { - bcopy(ea->ps, space, sizeof(void *) * ea->nelem); - kfree(ea->ps, M_DEVBUF); + /* + * Re-check under the lock; the allocation was done + * unlocked because M_INTWAIT may block. + */ + spin_lock(&ea->ex_lock); + if (index >= ea->nelem) { + /* Make sure we have something to copy before we + * copy it */ + if (ea->nelem) { + bcopy(ea->ps, space, + sizeof(void *) * ea->nelem); + } + oldps = ea->ps; + ea->ps = space; + ea->nelem = index + EXTEND_CHUNK; + spin_unlock(&ea->ex_lock); + if (oldps != NULL) + kfree(oldps, M_DEVBUF); + } else { + spin_unlock(&ea->ex_lock); + kfree(space, M_DEVBUF); } - - ea->ps = space; - ea->nelem = index + EXTEND_CHUNK; } + + spin_lock(&ea->ex_lock); if (ea->ps[index]) { + spin_unlock(&ea->ex_lock); kprintf("extend_set: entry %d already has storage.\n", index); - return 0; + return (NULL); } - else - ea->ps[index] = value; + ea->ps[index] = value; + spin_unlock(&ea->ex_lock); - return value; + return (value); } void * cam_extend_get(struct extend_array *ea, int index) { + void *p; + if (ea == NULL || index >= ea->nelem || index < 0) return NULL; - return ea->ps[index]; + spin_lock(&ea->ex_lock); + p = ea->ps[index]; + spin_unlock(&ea->ex_lock); + return (p); } void cam_extend_release(struct extend_array *ea, int index) { - void *p = cam_extend_get(ea, index); - if (p) { + if (ea == NULL || index >= ea->nelem || index < 0) + return; + spin_lock(&ea->ex_lock); + if (ea->ps[index] != NULL) { ea->ps[index] = NULL; } + spin_unlock(&ea->ex_lock); }