svn commit: r306710 - head/sys/cam

[Mark Johnston]

Author: markj
Date: Wed Oct  5 17:18:24 2016
New Revision: 306710
URL: https://svnweb.freebsd.org/changeset/base/306710

Log:
  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 <rlibby at gmail.com>
  Reviewed by:	imp, mav
  MFC after:	2 weeks
  Sponsored by:	Dell EMC Isilon
  Differential Revision:	https://reviews.freebsd.org/D8151

Modified:
  head/sys/cam/cam_queue.c
  head/sys/cam/cam_queue.h

Modified: head/sys/cam/cam_queue.c
==============================================================================
--- head/sys/cam/cam_queue.c	Wed Oct  5 17:04:58 2016	(r306709)
+++ head/sys/cam/cam_queue.c	Wed Oct  5 17:18:24 2016	(r306710)
@@ -176,8 +176,11 @@ camq_remove(struct camq *queue, int inde
 {
 	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];

Modified: head/sys/cam/cam_queue.h
==============================================================================
--- head/sys/cam/cam_queue.h	Wed Oct  5 17:04:58 2016	(r306709)
+++ head/sys/cam/cam_queue.h	Wed Oct  5 17:18:24 2016	(r306710)
@@ -197,6 +197,11 @@ cam_ccbq_insert_ccb(struct cam_ccbq *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 *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 *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