Fix the CTL OOA queue dumping code so that it does not hold a mutex

while doing a copyout.  That can cause a panic, because copyout
can trigger VM faults, and we can't handle VM faults while holding
a mutex.

The solution here is to malloc a separate buffer to hold the OOA
queue entries, so that we don't risk a VM fault while filling up
the buffer and we don't have to drop the lock.  The other solution
would be to wire the user's memory while filling their buffer with
copyout, but that would have been a little more complex.

Also fix a debugging parenthesis issue in ctl_abort_task() pointed
out by Chuck Tuffli.

Sponsored by:	Spectra Logic Corporation
MFC after:	1 week

1 files changed, 38 insertions, 25 deletions

diff --git a/sys/cam/ctl/ctl.c b/sys/cam/ctl/ctl.c
index 2d65fc8..ac5eba7 100644
--- a/sys/cam/ctl/ctl.c
+++ b/sys/cam/ctl/ctl.c
@@ -351,7 +351,8 @@ static void ctl_ioctl_hard_startstop_callback(void *arg,
 					      struct cfi_metatask *metatask);
 static void ctl_ioctl_bbrread_callback(void *arg,struct cfi_metatask *metatask);
 static int ctl_ioctl_fill_ooa(struct ctl_lun *lun, uint32_t *cur_fill_num,
-			      struct ctl_ooa *ooa_hdr);
+			      struct ctl_ooa *ooa_hdr,
+			      struct ctl_ooa_entry *kern_entries);
 static int ctl_ioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flag,
 		     struct thread *td);
 uint32_t ctl_get_resindex(struct ctl_nexus *nexus);
@@ -1960,7 +1961,7 @@ ctl_ioctl_bbrread_callback(void *arg, struct cfi_metatask *metatask)
  */
 static int
 ctl_ioctl_fill_ooa(struct ctl_lun *lun, uint32_t *cur_fill_num,
-		   struct ctl_ooa *ooa_hdr)
+		   struct ctl_ooa *ooa_hdr, struct ctl_ooa_entry *kern_entries)
 {
 	union ctl_io *io;
 	int retval;
@@ -1970,7 +1971,7 @@ ctl_ioctl_fill_ooa(struct ctl_lun *lun, uint32_t *cur_fill_num,
 	for (io = (union ctl_io *)TAILQ_FIRST(&lun->ooa_queue); (io != NULL);
 	     (*cur_fill_num)++, io = (union ctl_io *)TAILQ_NEXT(&io->io_hdr,
 	     ooa_links)) {
-		struct ctl_ooa_entry *cur_entry, entry;
+		struct ctl_ooa_entry *entry;
 
 		/*
 		 * If we've got more than we can fit, just count the
@@ -1979,36 +1980,29 @@ ctl_ioctl_fill_ooa(struct ctl_lun *lun, uint32_t *cur_fill_num,
 		if (*cur_fill_num >= ooa_hdr->alloc_num)
 			continue;
 
-		cur_entry = &ooa_hdr->entries[*cur_fill_num];
+		entry = &kern_entries[*cur_fill_num];
 
-		bzero(&entry, sizeof(entry));
-
-		entry.tag_num = io->scsiio.tag_num;
-		entry.lun_num = lun->lun;
+		entry->tag_num = io->scsiio.tag_num;
+		entry->lun_num = lun->lun;
 #ifdef CTL_TIME_IO
-		entry.start_bt = io->io_hdr.start_bt;
+		entry->start_bt = io->io_hdr.start_bt;
 #endif
-		bcopy(io->scsiio.cdb, entry.cdb, io->scsiio.cdb_len);
-		entry.cdb_len = io->scsiio.cdb_len;
+		bcopy(io->scsiio.cdb, entry->cdb, io->scsiio.cdb_len);
+		entry->cdb_len = io->scsiio.cdb_len;
 		if (io->io_hdr.flags & CTL_FLAG_BLOCKED)
-			entry.cmd_flags |= CTL_OOACMD_FLAG_BLOCKED;
+			entry->cmd_flags |= CTL_OOACMD_FLAG_BLOCKED;
 
 		if (io->io_hdr.flags & CTL_FLAG_DMA_INPROG)
-			entry.cmd_flags |= CTL_OOACMD_FLAG_DMA;
+			entry->cmd_flags |= CTL_OOACMD_FLAG_DMA;
 
 		if (io->io_hdr.flags & CTL_FLAG_ABORT)
-			entry.cmd_flags |= CTL_OOACMD_FLAG_ABORT;
+			entry->cmd_flags |= CTL_OOACMD_FLAG_ABORT;
 
 		if (io->io_hdr.flags & CTL_FLAG_IS_WAS_ON_RTR)
-			entry.cmd_flags |= CTL_OOACMD_FLAG_RTR;
+			entry->cmd_flags |= CTL_OOACMD_FLAG_RTR;
 
 		if (io->io_hdr.flags & CTL_FLAG_DMA_QUEUED)
-			entry.cmd_flags |= CTL_OOACMD_FLAG_DMA_QUEUED;
-
-		retval = copyout(&entry, cur_entry, sizeof(entry));
-
-		if (retval != 0)
-			break;
+			entry->cmd_flags |= CTL_OOACMD_FLAG_DMA_QUEUED;
 	}
 
 	return (retval);
@@ -2391,6 +2385,7 @@ ctl_ioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flag,
 	case CTL_GET_OOA: {
 		struct ctl_lun *lun;
 		struct ctl_ooa *ooa_hdr;
+		struct ctl_ooa_entry *entries;
 		uint32_t cur_fill_num;
 
 		ooa_hdr = (struct ctl_ooa *)addr;
@@ -2414,11 +2409,20 @@ ctl_ioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flag,
 			break;
 		}
 
+		entries = malloc(ooa_hdr->alloc_len, M_CTL, M_WAITOK | M_ZERO);
+		if (entries == NULL) {
+			printf("%s: could not allocate %d bytes for OOA "
+			       "dump\n", __func__, ooa_hdr->alloc_len);
+			retval = ENOMEM;
+			break;
+		}
+
 		mtx_lock(&softc->ctl_lock);
 		if (((ooa_hdr->flags & CTL_OOA_FLAG_ALL_LUNS) == 0)
 		 && ((ooa_hdr->lun_num > CTL_MAX_LUNS)
 		  || (softc->ctl_luns[ooa_hdr->lun_num] == NULL))) {
 			mtx_unlock(&softc->ctl_lock);
+			free(entries, M_CTL);
 			printf("%s: CTL_GET_OOA: invalid LUN %ju\n",
 			       __func__, (uintmax_t)ooa_hdr->lun_num);
 			retval = EINVAL;
@@ -2430,24 +2434,31 @@ ctl_ioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flag,
 		if (ooa_hdr->flags & CTL_OOA_FLAG_ALL_LUNS) {
 			STAILQ_FOREACH(lun, &softc->lun_list, links) {
 				retval = ctl_ioctl_fill_ooa(lun, &cur_fill_num,
-					ooa_hdr);
+					ooa_hdr, entries);
 				if (retval != 0)
 					break;
 			}
 			if (retval != 0) {
 				mtx_unlock(&softc->ctl_lock);
+				free(entries, M_CTL);
 				break;
 			}
 		} else {
 			lun = softc->ctl_luns[ooa_hdr->lun_num];
 
-			retval = ctl_ioctl_fill_ooa(lun, &cur_fill_num,ooa_hdr);
+			retval = ctl_ioctl_fill_ooa(lun, &cur_fill_num,ooa_hdr,
+						    entries);
 		}
 		mtx_unlock(&softc->ctl_lock);
 
 		ooa_hdr->fill_num = min(cur_fill_num, ooa_hdr->alloc_num);
 		ooa_hdr->fill_len = ooa_hdr->fill_num *
 			sizeof(struct ctl_ooa_entry);
+		retval = copyout(entries, ooa_hdr->entries, ooa_hdr->fill_len);
+		if (retval != 0) {
+			printf("%s: error copying out %d bytes for OOA dump\n", 
+			       __func__, ooa_hdr->fill_len);
+		}
 
 		getbintime(&ooa_hdr->cur_bt);
 
@@ -2458,6 +2469,8 @@ ctl_ioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flag,
 			ooa_hdr->dropped_num = 0;
 			ooa_hdr->status = CTL_OOA_OK;
 		}
+
+		free(entries, M_CTL);
 		break;
 	}
 	case CTL_CHECK_OOA: {
@@ -10782,9 +10795,9 @@ ctl_abort_task(union ctl_io *io)
 			    (xio->io_hdr.flags &
 			    CTL_FLAG_DMA_INPROG) ? " DMA" : "",
 			    (xio->io_hdr.flags &
-			    CTL_FLAG_ABORT) ? " ABORT" : ""),
+			    CTL_FLAG_ABORT) ? " ABORT" : "",
 			    (xio->io_hdr.flags &
-			    CTL_FLAG_IS_WAS_ON_RTR ? " RTR" : "");
+			    CTL_FLAG_IS_WAS_ON_RTR ? " RTR" : ""));
 		ctl_scsi_command_string(&xio->scsiio, NULL, &sb);
 		sbuf_finish(&sb);
 		printf("%s\n", sbuf_data(&sb));