Probable fix for the "cdda2wav" panics that various people have been

reporting since this past summer.  (I think Daniel O'Conner was the first.)

The problem appears to have been something like this:

 - cdda2wav by default passes in a buffer that is close to the 128K MAXPHYS
   limit.
 - many times, the buffer is not page aligned
 - vmapbuf() truncates the address, so that it is page aligned
 - that causes the total size of the buffer to be greater than MAXPHYS,
   which of course is a bad thing.

Here's a quote from the PR (kern/9067):

==================
In particular, note bp->b_bufsize = 0x0001f950 and bp->b_data = 0xf2219960
(which does not start on a page boundary).  vunmapbuf() loops through all
the pages without any difficulty until addr reaches 0xf2239000, and then
the panic occurs.  This seems to indicate that we are exceeding MAXPHYS
since we actually started from the middle of a page (the data is being
transfered to a non page aligned location).

To complete the description, note that the system call originates from
ReadCddaMMC12() (in scsi_cmds.c of cdda2wav) with a request to read 55
audio sectors of 2352 bytes (which is calculated to fall under MAXPHYS).
This in turn ends up calling scsi_send() (in scsi-bsd.c) which calls
cam_fill_csio() and cam_send_ccb().  This results in a CAMIOCOMMAND ioctl
with a ccb function code of XPT_SCSI_IO.
==================

The fix is to change the size check in cam_periph_mapmem() so that it is
like the one in minphys().  In particular, it is something like:

if ((buffer_length + (buf_ptr & PAGE_MASK)) > MAXPHYS)
	buffer is too big

My fix is based on the one in the PR, but I cleaned up a fair number of
things in cam_periph_mapmem().  The checks for each buffer to be mapped
are now in a separate loop from the actual mapping operation.  With the new
arrangement, we don't have to bother with unmapping any previously mapped
buffers if one of the checks fails.

Many thanks to James Liu for tracking this down.  I'd appreciate it if some
vm-savvy folks would look this over.  I believe this fix is correct, but I
could be wrong.

PR:		kern/9067 (also, kern/8112)
Reviewed by:	gibbs
Submitted by:	"James T. Liu" <jtliu@phlebas.rockefeller.edu>

1 files changed, 39 insertions, 47 deletions

diff --git a/sys/cam/cam_periph.c b/sys/cam/cam_periph.c
index 70ab1b1..1a707601 100644
--- a/sys/cam/cam_periph.c
+++ b/sys/cam/cam_periph.c
@@ -26,7 +26,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- *      $Id: cam_periph.c,v 1.5 1998/10/15 17:46:18 ken Exp $
+ *      $Id: cam_periph.c,v 1.6 1998/10/22 22:16:48 ken Exp $
  */
 
 #include <sys/param.h>
@@ -490,24 +490,14 @@ cam_periph_unlock(struct cam_periph *periph)
 int
 cam_periph_mapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo)
 {
-	int flags, numbufs, i;
+	int numbufs, i;
+	int flags[CAM_PERIPH_MAXMAPS];
 	u_int8_t **data_ptrs[CAM_PERIPH_MAXMAPS];
 	u_int32_t lengths[CAM_PERIPH_MAXMAPS];
 	u_int32_t dirs[CAM_PERIPH_MAXMAPS];
 
 	switch(ccb->ccb_h.func_code) {
 	case XPT_DEV_MATCH:
-		if (ccb->cdm.pattern_buf_len > MAXPHYS) {
-			printf("cam_periph_mapmem: attempt to map %u bytes, "
-			       "which is greater than MAXPHYS(%d)\n",
-			       ccb->cdm.pattern_buf_len, MAXPHYS);
-			return(E2BIG);
-		} else if (ccb->cdm.match_buf_len > MAXPHYS) {
-			printf("cam_periph_mapmem: attempt to map %u bytes, "
-			       "which is greater than MAXPHYS(%d)\n",
-			       ccb->cdm.match_buf_len, MAXPHYS);
-			return(E2BIG);
-		}
 		if (ccb->cdm.match_buf_len == 0) {
 			printf("cam_periph_mapmem: invalid match buffer "
 			       "length 0\n");
@@ -529,18 +519,11 @@ cam_periph_mapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo)
 		}
 		break;
 	case XPT_SCSI_IO:
-		if (ccb->csio.dxfer_len > MAXPHYS) {
-			printf("cam_periph_mapmem: attempt to map %u bytes, "
-			       "which is greater than MAXPHYS(%d)\n",
-				ccb->csio.dxfer_len, MAXPHYS);
-			return(E2BIG);
-		}
-
 		if ((ccb->ccb_h.flags & CAM_DIR_MASK) == CAM_DIR_NONE)
 			return(0);
 
 		data_ptrs[0] = &ccb->csio.data_ptr;
-		lengths[0] = ccb->csio.dxfer_len;;
+		lengths[0] = ccb->csio.dxfer_len;
 		dirs[0] = ccb->ccb_h.flags & CAM_DIR_MASK;
 		numbufs = 1;
 		break;
@@ -549,32 +532,40 @@ cam_periph_mapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo)
 		break; /* NOTREACHED */
 	}
 
-	/* this keeps the current process from getting swapped */
 	/*
-	 * XXX KDM should I use P_NOSWAP instead?
+	 * Check the transfer length and permissions first, so we don't
+	 * have to unmap any previously mapped buffers.
 	 */
-	curproc->p_flag |= P_PHYSIO;
-
 	for (i = 0; i < numbufs; i++) {
-		flags = 0;
+
+		flags[i] = 0;
+
+		/*
+		 * The userland data pointer passed in may not be page
+		 * aligned.  vmapbuf() truncates the address to a page
+		 * boundary, so if the address isn't page aligned, we'll
+		 * need enough space for the given transfer length, plus
+		 * whatever extra space is necessary to make it to the page
+		 * boundary.
+		 */
+		if ((lengths[i] +
+		    (((vm_offset_t)(*data_ptrs[i])) & PAGE_MASK)) > MAXPHYS){
+			printf("cam_periph_mapmem: attempt to map %u bytes, "
+			       "which is greater than MAXPHYS(%d)\n",
+			       lengths[i] +
+			       (((vm_offset_t)(*data_ptrs[i])) & PAGE_MASK),
+			       MAXPHYS);
+			return(E2BIG);
+		}
 
 		if (dirs[i] & CAM_DIR_IN) {
-			flags = B_READ;
+			flags[i] = B_READ;
 			if (useracc(*data_ptrs[i], lengths[i], B_READ) == 0){
 				printf("cam_periph_mapmem: error, "
 					"address %p, length %lu isn't "
 					"user accessible for READ\n",
 					(void *)*data_ptrs[i],
 					(u_long)lengths[i]);
-				/*
-				 * If we've already mapped one or more
-				 * buffers for this CCB, unmap it (them).
-				 */
-				if (i > 0)
-					cam_periph_unmapmem(ccb, mapinfo);
-				else
-					curproc->p_flag &= ~P_PHYSIO;
-
 				return(EACCES);
 			}
 		}
@@ -584,26 +575,27 @@ cam_periph_mapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo)
 		 * is all 0's, and so it is "set" all the time.
 		 */
 		if (dirs[i] & CAM_DIR_OUT) {
-			flags |= B_WRITE;
+			flags[i] |= B_WRITE;
 			if (useracc(*data_ptrs[i], lengths[i], B_WRITE) == 0){
 				printf("cam_periph_mapmem: error, "
 					"address %p, length %lu isn't "
 					"user accessible for WRITE\n",
 					(void *)*data_ptrs[i],
 					(u_long)lengths[i]);
-				/*
-				 * If we've already mapped one or more
-				 * buffers for this CCB, unmap it (them).
-				 */
-				if (i > 0)
-					cam_periph_unmapmem(ccb, mapinfo);
-				else
-					curproc->p_flag &= ~P_PHYSIO;
 
 				return(EACCES);
 			}
 		}
 
+	}
+
+	/* this keeps the current process from getting swapped */
+	/*
+	 * XXX KDM should I use P_NOSWAP instead?
+	 */
+	curproc->p_flag |= P_PHYSIO;
+
+	for (i = 0; i < numbufs; i++) {
 		/*
 		 * Get the buffer.
 		 */
@@ -615,11 +607,11 @@ cam_periph_mapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo)
 		/* put our pointer in the data slot */
 		mapinfo->bp[i]->b_data = *data_ptrs[i];
 
-		/* set the transfer length, we know it's < 64K */
+		/* set the transfer length, we know it's < MAXPHYS */
 		mapinfo->bp[i]->b_bufsize = lengths[i];
 
 		/* set the flags */
-		mapinfo->bp[i]->b_flags = flags | B_PHYS | B_BUSY;
+		mapinfo->bp[i]->b_flags = flags[i] | B_PHYS | B_BUSY;
 
 		/* map the buffer into kernel memory */
 		vmapbuf(mapinfo->bp[i]);