svn commit: r264229 - head/sys/dev/mps

[Scott Long]

Author: scottl
Date: Mon Apr  7 15:13:24 2014
New Revision: 264229
URL: http://svnweb.freebsd.org/changeset/base/264229

Log:
  Add some assertions to ensure that the target array doesn't get accessed
  out of bounds.
  
  Obtained from:	Netflix, Inc.
  MFC after:	3 days

Modified:
  head/sys/dev/mps/mps_sas.c
  head/sys/dev/mps/mps_sas.h

Modified: head/sys/dev/mps/mps_sas.c
==============================================================================
--- head/sys/dev/mps/mps_sas.c	Mon Apr  7 13:03:58 2014	(r264228)
+++ head/sys/dev/mps/mps_sas.c	Mon Apr  7 15:13:24 2014	(r264229)
@@ -154,7 +154,7 @@ mpssas_find_target_by_handle(struct mpss
 	struct mpssas_target *target;
 	int i;
 
-	for (i = start; i < sassc->sc->facts->MaxTargets; i++) {
+	for (i = start; i < sassc->maxtargets; i++) {
 		target = &sassc->targets[i];
 		if (target->handle == handle)
 			return (target);
@@ -709,8 +709,16 @@ mps_attach_sas(struct mps_softc *sc)
 		__func__, __LINE__);
 		return (ENOMEM);
 	}
+
+	/*
+	 * XXX MaxTargets could change during a reinit.  Since we don't
+	 * resize the targets[] array during such an event, cache the value
+	 * of MaxTargets here so that we don't get into trouble later.  This
+	 * should move into the reinit logic.
+	 */
+	sassc->maxtargets = sc->facts->MaxTargets;
 	sassc->targets = malloc(sizeof(struct mpssas_target) *
-	    sc->facts->MaxTargets, M_MPT2, M_WAITOK|M_ZERO);
+	    sassc->maxtargets, M_MPT2, M_WAITOK|M_ZERO);
 	if(!sassc->targets) {
 		device_printf(sc->mps_dev, "Cannot allocate memory %s %d\n",
 		__func__, __LINE__);
@@ -868,7 +876,7 @@ mps_detach_sas(struct mps_softc *sc)
 	if (sassc->devq != NULL)
 		cam_simq_free(sassc->devq);
 
-	for(i=0; i< sc->facts->MaxTargets ;i++) {
+	for(i=0; i< sassc->maxtargets ;i++) {
 		targ = &sassc->targets[i];
 		SLIST_FOREACH_SAFE(lun, &targ->luns, lun_link, lun_tmp) {
 			free(lun, M_MPT2);
@@ -959,9 +967,9 @@ mpssas_action(struct cam_sim *sim, union
 		cpi->hba_misc = PIM_NOBUSRESET | PIM_UNMAPPED;
 #endif
 		cpi->hba_eng_cnt = 0;
-		cpi->max_target = sassc->sc->facts->MaxTargets - 1;
+		cpi->max_target = sassc->maxtargets - 1;
 		cpi->max_lun = 255;
-		cpi->initiator_id = sassc->sc->facts->MaxTargets - 1;
+		cpi->initiator_id = sassc->maxtargets - 1;
 		strncpy(cpi->sim_vid, "FreeBSD", SIM_IDLEN);
 		strncpy(cpi->hba_vid, "LSILogic", HBA_IDLEN);
 		strncpy(cpi->dev_name, cam_sim_name(sim), DEV_IDLEN);
@@ -992,6 +1000,9 @@ mpssas_action(struct cam_sim *sim, union
 		sas = &cts->xport_specific.sas;
 		scsi = &cts->proto_specific.scsi;
 
+		KASSERT(cts->ccb_h.target_id < sassc->maxtargets,
+		    ("Target %d out of bounds in XPT_GET_TRANS_SETTINGS\n",
+		    cts->ccb_h.target_id));
 		targ = &sassc->targets[cts->ccb_h.target_id];
 		if (targ->handle == 0x0) {
 			cts->ccb_h.status = CAM_SEL_TIMEOUT;
@@ -1155,7 +1166,7 @@ mpssas_handle_reinit(struct mps_softc *s
 	 * reset, and we have to rediscover all the targets and use the new
 	 * handles.  
 	 */
-	for (i = 0; i < sc->facts->MaxTargets; i++) {
+	for (i = 0; i < sc->sassc->maxtargets; i++) {
 		if (sc->sassc->targets[i].outstanding != 0)
 			mps_dprint(sc, MPS_INIT, "target %u outstanding %u\n", 
 			    i, sc->sassc->targets[i].outstanding);
@@ -1631,6 +1642,9 @@ mpssas_action_scsiio(struct mpssas_softc
 	mtx_assert(&sc->mps_mtx, MA_OWNED);
 
 	csio = &ccb->csio;
+	KASSERT(csio->ccb_h.target_id < sassc->maxtargets,
+	    ("Target %d out of bounds in XPT_SCSI_IO\n",
+	     csio->ccb_h.target_id));
 	targ = &sassc->targets[csio->ccb_h.target_id];
 	mps_dprint(sc, MPS_TRACE, "ccb %p target flag %x\n", ccb, targ->flags);
 	if (targ->handle == 0x0) {
@@ -2929,6 +2943,8 @@ mpssas_action_smpio(struct mpssas_softc 
 	/*
 	 * Make sure the target exists.
 	 */
+	KASSERT(ccb->ccb_h.target_id < sassc->maxtargets,
+	    ("Target %d out of bounds in XPT_SMP_IO\n", ccb->ccb_h.target_id));
 	targ = &sassc->targets[ccb->ccb_h.target_id];
 	if (targ->handle == 0x0) {
 		mps_dprint(sc, MPS_ERROR,
@@ -3063,6 +3079,9 @@ mpssas_action_resetdev(struct mpssas_sof
 	MPS_FUNCTRACE(sassc->sc);
 	mtx_assert(&sassc->sc->mps_mtx, MA_OWNED);
 
+	KASSERT(ccb->ccb_h.target_id < sassc->maxtargets,
+	    ("Target %d out of bounds in XPT_RESET_DEV\n",
+	     ccb->ccb_h.target_id));
 	sc = sassc->sc;
 	tm = mps_alloc_command(sc);
 	if (tm == NULL) {
@@ -3191,6 +3210,9 @@ mpssas_async(void *callback_arg, uint32_
 		/*
 		 * We should have a handle for this, but check to make sure.
 		 */
+		KASSERT(xpt_path_target_id(path) < sassc->maxtargets,
+		    ("Target %d out of bounds in mpssas_async\n",
+		    xpt_path_target_id(path)));
 		target = &sassc->targets[xpt_path_target_id(path)];
 		if (target->handle == 0)
 			break;
@@ -3278,6 +3300,9 @@ mpssas_check_eedp(struct mps_softc *sc, 
 	targetid = xpt_path_target_id(path);
 	lunid = xpt_path_lun_id(path);
 
+	KASSERT(targetid < sassc->maxtargets,
+	    ("Target %d out of bounds in mpssas_check_eedp\n",
+	     targetid));
 	target = &sassc->targets[targetid];
 	if (target->handle == 0x0)
 		return;
@@ -3413,6 +3438,9 @@ mpssas_read_cap_done(struct cam_periph *
 	 * target.
 	 */
 	sassc = (struct mpssas_softc *)done_ccb->ccb_h.ppriv_ptr1;
+	KASSERT(done_ccb->ccb_h.target_id < sassc->maxtargets,
+	    ("Target %d out of bounds in mpssas_read_cap_done\n",
+	     done_ccb->ccb_h.target_id));
 	target = &sassc->targets[done_ccb->ccb_h.target_id];
 	SLIST_FOREACH(lun, &target->luns, lun_link) {
 		if (lun->lun_id != done_ccb->ccb_h.target_lun)

Modified: head/sys/dev/mps/mps_sas.h
==============================================================================
--- head/sys/dev/mps/mps_sas.h	Mon Apr  7 13:03:58 2014	(r264228)
+++ head/sys/dev/mps/mps_sas.h	Mon Apr  7 15:13:24 2014	(r264229)
@@ -87,6 +87,7 @@ struct mpssas_softc {
 #define MPSSAS_DISCOVERY_TIMEOUT_PENDING	(1 << 2)
 #define MPSSAS_QUEUE_FROZEN	(1 << 3)
 #define	MPSSAS_SHUTDOWN		(1 << 4)
+	u_int			maxtargets;
 	struct mpssas_target	*targets;
 	struct cam_devq		*devq;
 	struct cam_sim		*sim;