--- a/sys/bus/cam/cam_periph.c +++ b/sys/bus/cam/cam_periph.c @@ -169,15 +169,45 @@ cam_periph_alloc(periph_ctor_t *periph_ctor, init_level = 0; /* * Handle Hot-Plug scenarios. If there is already a peripheral * of our type assigned to this path, we are likely waiting for * final close on an old, invalidated, peripheral. If this is * the case, queue up a deferred call to the peripheral's async * handler. If it looks like a mistaken re-allocation, complain. + * + * DF-5328: cam_periph_find() returns a borrowed pointer after + * dropping the buses lock; touching periph->flags / deferred_* + * unlocked races camperiphfree() (buses-lock-held TAILQ_REMOVE, + * dtor, kfree). Perform the lookup AND the deferred bookkeeping + * under one buses-lock hold so it serializes with camperiphfree(): + * either the periph was already removed from the units list (we + * see NULL), or our stores complete before camperiphfree() reads + * the flag at the top of its deferred-callback block. Store + * deferred_callback/deferred_ac BEFORE the NEW_DEV_FOUND flag so + * camperiphfree() can never observe the flag with an unpublished + * (NULL) callback pointer. */ - if ((periph = cam_periph_find(path, name)) != NULL) { + xpt_lock_buses(); + periph = NULL; + for (p_drv = periph_drivers; *p_drv != NULL; p_drv++) { + if (strcmp((*p_drv)->driver_name, name) == 0) + break; + } + if (*p_drv != NULL) { + TAILQ_FOREACH(periph, &(*p_drv)->units, unit_links) { + if (xpt_path_comp(periph->path, path) == 0) + break; + } + } + if (periph != NULL) { if ((periph->flags & CAM_PERIPH_INVALID) != 0 && (periph->flags & CAM_PERIPH_NEW_DEV_FOUND) == 0) { - periph->flags |= CAM_PERIPH_NEW_DEV_FOUND; periph->deferred_callback = ac_callback; periph->deferred_ac = code; + cpu_sfence(); + periph->flags |= CAM_PERIPH_NEW_DEV_FOUND; + xpt_unlock_buses(); return (CAM_REQ_INPROG); } else { + xpt_unlock_buses(); kprintf("cam_periph_alloc: attempt to re-allocate " "valid device %s%d rejected\n", periph->periph_name, periph->unit_number); + return (CAM_REQ_INVALID); } - return (CAM_REQ_INVALID); } + xpt_unlock_buses(); --- a/sys/bus/cam/cam_periph.c +++ b/sys/bus/cam/ciphelper_note @@ -299,6 +299,11 @@ cam_periph_find(struct cam_path *path, char *name) xpt_unlock_buses(); return(NULL); } + +/* + * NB: cam_periph_find() drops the buses lock before returning. The + * returned periph is borrowed: a caller that keeps using it must hold + * its own reference or perform the lookup under the buses lock (see + * cam_periph_alloc()). + */