From f30cad3355435ba521be57d4415d810983f8ce04 Mon Sep 17 00:00:00 2001 From: Mark Johnston Date: Wed, 5 Oct 2016 17:18:24 +0000 Subject: [PATCH] CAM ccbq sanity: checks on insert and remove KASSERT in cam_ccbq_insert_ccb that only XPT_FC_QUEUED ops are queued, and XPT_FC_USER_CCB ops are not. Otherwise cam_ccbq_ccb_done may be skipped. Bounds check the index used for camq_remove in order to panic instead of scribble on removal of an out-of-bounds index (e.g. consider the effect of camq_remove of CAM_UNQUEUED_INDEX). KASSERT in cam_ccbq_remove_ccb that the ccb removed by index was the one sought. Submitted by: Ryan Libby Reviewed by: imp, mav MFC after: 2 weeks Sponsored by: Dell EMC Isilon Differential Revision: https://reviews.freebsd.org/D8151 --- sys/cam/cam_queue.c | 7 +++++-- sys/cam/cam_queue.h | 11 ++++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/sys/cam/cam_queue.c b/sys/cam/cam_queue.c index 3959c3d64098..059dd63ca649 100644 --- a/sys/cam/cam_queue.c +++ b/sys/cam/cam_queue.c @@ -176,8 +176,11 @@ camq_remove(struct camq *queue, int index) { cam_pinfo *removed_entry; - if (index == 0 || index > queue->entries) - return (NULL); + if (index <= 0 || index > queue->entries) + panic("%s: Attempt to remove out-of-bounds index %d " + "from queue %p of size %d", __func__, index, queue, + queue->entries); + removed_entry = queue->queue_array[index]; if (queue->entries != index) { queue->queue_array[index] = queue->queue_array[queue->entries]; diff --git a/sys/cam/cam_queue.h b/sys/cam/cam_queue.h index 33f6b1d979fc..b15998b5c7fa 100644 --- a/sys/cam/cam_queue.h +++ b/sys/cam/cam_queue.h @@ -197,6 +197,11 @@ cam_ccbq_insert_ccb(struct cam_ccbq *ccbq, union ccb *new_ccb) struct ccb_hdr *old_ccb; struct camq *queue = &ccbq->queue; + KASSERT((new_ccb->ccb_h.func_code & XPT_FC_QUEUED) != 0 && + (new_ccb->ccb_h.func_code & XPT_FC_USER_CCB) == 0, + ("%s: Cannot queue ccb %p func_code %#x", __func__, new_ccb, + new_ccb->ccb_h.func_code)); + /* * If queue is already full, try to resize. * If resize fail, push CCB with lowest priority out to the TAILQ. @@ -218,6 +223,7 @@ cam_ccbq_remove_ccb(struct cam_ccbq *ccbq, union ccb *ccb) { struct ccb_hdr *cccb, *bccb; struct camq *queue = &ccbq->queue; + cam_pinfo *removed_entry __unused; /* If the CCB is on the TAILQ, remove it from there. */ if (ccb->ccb_h.pinfo.index == CAM_EXTRAQ_INDEX) { @@ -228,7 +234,10 @@ cam_ccbq_remove_ccb(struct cam_ccbq *ccbq, union ccb *ccb) return; } - camq_remove(queue, ccb->ccb_h.pinfo.index); + removed_entry = camq_remove(queue, ccb->ccb_h.pinfo.index); + KASSERT(removed_entry == &ccb->ccb_h.pinfo, + ("%s: Removed wrong entry from queue (%p != %p)", __func__, + removed_entry, &ccb->ccb_h.pinfo)); /* * If there are some CCBs on TAILQ, find the best one and move it