cam: Let cam_periph_unmapmem() return an error

As of commit b059686a71c8, cam_periph_unmapmem() can legitimately fail
if the copyout() operation fails.  However, this failure was never
signaled to upper layers.  In practice it is unlikely to occur
since cap_periph_mapmem() would most likely fail in such
circumstances anyway, but an error is nonetheless possible.

However, some code reading revealed a few paths where the return value
of cam_periph_mapmem() is not checked, and this is definitely a bug.
Add error checking there and let cam_periph_unmapmem() return errors
from copyout().

Reviewed by:	dab, mav
MFC after:	2 weeks
Differential Revision:	https://reviews.freebsd.org/D43201

(cherry picked from commit d068ea16e3264c2d62472a8acf794262cfe703dd)

8 files changed, 57 insertions, 35 deletions

diff --git a/sys/cam/cam_compat.c b/sys/cam/cam_compat.c
index c377763fc7c5..aed3916267d5 100644
--- a/sys/cam/cam_compat.c
+++ b/sys/cam/cam_compat.c
@@ -380,11 +380,13 @@ cam_compat_translate_dev_match_0x18(union ccb *ccb)
 	struct dev_match_result		*dm;
 	struct dev_match_result_0x18	*dm18;
 	struct cam_periph_map_info	mapinfo;
-	int i;
+	int error, i;
 
 	/* Remap the CCB into kernel address space */
 	bzero(&mapinfo, sizeof(mapinfo));
-	cam_periph_mapmem(ccb, &mapinfo, maxphys);
+	error = cam_periph_mapmem(ccb, &mapinfo, maxphys);
+	if (error != 0)
+		return (error);
 
 	dm = ccb->cdm.matches;
 	/* Translate in-place: old fields are smaller */
@@ -432,21 +434,22 @@ cam_compat_translate_dev_match_0x18(union ccb *ccb)
 		}
 	}
 
-	cam_periph_unmapmem(ccb, &mapinfo);
-
-	return (0);
+	return (cam_periph_unmapmem(ccb, &mapinfo));
 }
 
 static int
 cam_compat_handle_0x19(struct cdev *dev, u_long cmd, caddr_t addr, int flag,
     struct thread *td, d_ioctl_t *cbfnp)
 {
+	struct cam_periph_map_info mapinfo;
 	union ccb *ccb = (union ccb *)addr;
-	struct cam_periph_map_info	mapinfo;
+	int error;
 
 	if (cmd == CAMIOCOMMAND && ccb->ccb_h.func_code == XPT_DEV_MATCH) {
 		bzero(&mapinfo, sizeof(mapinfo));
-		cam_periph_mapmem(ccb, &mapinfo, maxphys);
+		error = cam_periph_mapmem(ccb, &mapinfo, maxphys);
+		if (error != 0)
+			return (error);
 		for (int i = 0; i < ccb->cdm.num_patterns; i++) {
 			struct dev_match_pattern *p = &ccb->cdm.patterns[i];
 
@@ -460,7 +463,9 @@ cam_compat_handle_0x19(struct cdev *dev, u_long cmd, caddr_t addr, int flag,
 			    p->pattern.periph_pattern.flags == 0x01f)
 				p->pattern.periph_pattern.flags = PERIPH_MATCH_ANY;
 		}
-		cam_periph_unmapmem(ccb, &mapinfo);
+		error = cam_periph_unmapmem(ccb, &mapinfo);
+		if (error != 0)
+			return (error);
 	}
 	return ((cbfnp)(dev, cmd, addr, flag, td));
 }
diff --git a/sys/cam/cam_periph.c b/sys/cam/cam_periph.c
index ebdda9ef449f..af187d3eab51 100644
--- a/sys/cam/cam_periph.c
+++ b/sys/cam/cam_periph.c
@@ -1014,17 +1014,17 @@ fail:
  * Unmap memory segments mapped into kernel virtual address space by
  * cam_periph_mapmem().
  */
-void
+int
 cam_periph_unmapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo)
 {
-	int numbufs, i;
+	int error, numbufs, i;
 	uint8_t **data_ptrs[CAM_PERIPH_MAXMAPS];
 	uint32_t lengths[CAM_PERIPH_MAXMAPS];
 	uint32_t dirs[CAM_PERIPH_MAXMAPS];
 
 	if (mapinfo->num_bufs_used <= 0) {
 		/* nothing to free and the process wasn't held. */
-		return;
+		return (0);
 	}
 
 	switch (ccb->ccb_h.func_code) {
@@ -1089,12 +1089,11 @@ cam_periph_unmapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo)
 		numbufs = 1;
 		break;
 	default:
-		/* allow ourselves to be swapped once again */
-		PRELE(curproc);
-		return;
-		break; /* NOTREACHED */ 
+		numbufs = 0;
+		break;
 	}
 
+	error = 0;
 	for (i = 0; i < numbufs; i++) {
 		if (mapinfo->bp[i]) {
 			/* unmap the buffer */
@@ -1104,8 +1103,12 @@ cam_periph_unmapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo)
 			uma_zfree(pbuf_zone, mapinfo->bp[i]);
 		} else {
 			if (dirs[i] != CAM_DIR_OUT) {
-				copyout(*data_ptrs[i], mapinfo->orig[i],
+				int error1;
+
+				error1 = copyout(*data_ptrs[i], mapinfo->orig[i],
 				    lengths[i]);
+				if (error == 0)
+					error = error1;
 			}
 			free(*data_ptrs[i], M_CAMPERIPH);
 		}
@@ -1116,6 +1119,8 @@ cam_periph_unmapmem(union ccb *ccb, struct cam_periph_map_info *mapinfo)
 
 	/* allow ourselves to be swapped once again */
 	PRELE(curproc);
+
+	return (error);
 }
 
 int
diff --git a/sys/cam/cam_periph.h b/sys/cam/cam_periph.h
index d0f04388b181..ad18ee6b204c 100644
--- a/sys/cam/cam_periph.h
+++ b/sys/cam/cam_periph.h
@@ -180,7 +180,7 @@ void		cam_periph_invalidate(struct cam_periph *periph);
 int		cam_periph_mapmem(union ccb *ccb,
 				  struct cam_periph_map_info *mapinfo,
 				  u_int maxmap);
-void		cam_periph_unmapmem(union ccb *ccb,
+int		cam_periph_unmapmem(union ccb *ccb,
 				    struct cam_periph_map_info *mapinfo);
 union ccb	*cam_periph_getccb(struct cam_periph *periph,
 				   uint32_t priority);
diff --git a/sys/cam/cam_xpt.c b/sys/cam/cam_xpt.c
index 26e656fc5ff2..6eac81c18160 100644
--- a/sys/cam/cam_xpt.c
+++ b/sys/cam/cam_xpt.c
@@ -566,11 +566,9 @@ xptdoioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flag, struct thread *
 			/*
 			 * Map the buffers back into user space.
 			 */
-			cam_periph_unmapmem(inccb, &mapinfo);
+			error = cam_periph_unmapmem(inccb, &mapinfo);
 
 			inccb->ccb_h.path = old_path;
-
-			error = 0;
 			break;
 		}
 		default:
diff --git a/sys/cam/nvme/nvme_da.c b/sys/cam/nvme/nvme_da.c
index e8e75a121679..9901ecccf21c 100644
--- a/sys/cam/nvme/nvme_da.c
+++ b/sys/cam/nvme/nvme_da.c
@@ -438,8 +438,9 @@ ndaioctl(struct disk *dp, u_long cmd, void *data, int fflag,
 		 * Tear down mapping and return status.
 		 */
 		cam_periph_unlock(periph);
-		cam_periph_unmapmem(ccb, &mapinfo);
-		error = cam_ccb_success(ccb) ? 0 : EIO;
+		error = cam_periph_unmapmem(ccb, &mapinfo);
+		if (!cam_ccb_success(ccb))
+			error = EIO;
 out:
 		cam_periph_lock(periph);
 		xpt_release_ccb(ccb);
diff --git a/sys/cam/scsi/scsi_pass.c b/sys/cam/scsi/scsi_pass.c
index cf27c304d919..ff48bed30e68 100644
--- a/sys/cam/scsi/scsi_pass.c
+++ b/sys/cam/scsi/scsi_pass.c
@@ -2237,14 +2237,14 @@ passsendccb(struct cam_periph *periph, union ccb *ccb, union ccb *inccb)
 	}
 
 	cam_periph_unlock(periph);
-	cam_periph_unmapmem(ccb, &mapinfo);
+	error = cam_periph_unmapmem(ccb, &mapinfo);
 	cam_periph_lock(periph);
 
 	ccb->ccb_h.cbfcnp = NULL;
 	ccb->ccb_h.periph_priv = inccb->ccb_h.periph_priv;
 	bcopy(ccb, inccb, sizeof(union ccb));
 
-	return(0);
+	return (error);
 }
 
 /*
diff --git a/sys/cam/scsi/scsi_sg.c b/sys/cam/scsi/scsi_sg.c
index b17a156503a2..66ea4b4aa2d7 100644
--- a/sys/cam/scsi/scsi_sg.c
+++ b/sys/cam/scsi/scsi_sg.c
@@ -883,7 +883,7 @@ sgsendccb(struct cam_periph *periph, union ccb *ccb)
 {
 	struct sg_softc *softc;
 	struct cam_periph_map_info mapinfo;
-	int error;
+	int error, error1;
 
 	softc = periph->softc;
 	bzero(&mapinfo, sizeof(mapinfo));
@@ -908,7 +908,9 @@ sgsendccb(struct cam_periph *periph, union ccb *ccb)
 				  softc->device_stats);
 
 	cam_periph_unlock(periph);
-	cam_periph_unmapmem(ccb, &mapinfo);
+	error1 = cam_periph_unmapmem(ccb, &mapinfo);
+	if (error == 0)
+		error = error1;
 	cam_periph_lock(periph);
 
 	return (error);
diff --git a/sys/cam/scsi/scsi_target.c b/sys/cam/scsi/scsi_target.c
index 45a5eb8415b9..6b2fd5adb4ee 100644
--- a/sys/cam/scsi/scsi_target.c
+++ b/sys/cam/scsi/scsi_target.c
@@ -906,18 +906,29 @@ targreturnccb(struct targ_softc *softc, union ccb *ccb)
 	u_ccbh = &descr->user_ccb->ccb_h;
 
 	/* Copy out the central portion of the ccb_hdr */
-	copyout(&ccb->ccb_h.retry_count, &u_ccbh->retry_count,
-		offsetof(struct ccb_hdr, periph_priv) -
-		offsetof(struct ccb_hdr, retry_count));
+	error = copyout(&ccb->ccb_h.retry_count, &u_ccbh->retry_count,
+	    offsetof(struct ccb_hdr, periph_priv) -
+	    offsetof(struct ccb_hdr, retry_count));
+	if (error != 0) {
+		xpt_print(softc->path,
+		    "targreturnccb - CCB header copyout failed (%d)\n", error);
+	}
 
 	/* Copy out the rest of the ccb (after the ccb_hdr) */
 	ccb_len = targccblen(ccb->ccb_h.func_code) - sizeof(struct ccb_hdr);
-	if (descr->mapinfo.num_bufs_used != 0)
-		cam_periph_unmapmem(ccb, &descr->mapinfo);
-	error = copyout(&ccb->ccb_h + 1, u_ccbh + 1, ccb_len);
-	if (error != 0) {
-		xpt_print(softc->path,
-		    "targreturnccb - CCB copyout failed (%d)\n", error);
+	if (descr->mapinfo.num_bufs_used != 0) {
+		int error1;
+
+		error1 = cam_periph_unmapmem(ccb, &descr->mapinfo);
+		if (error == 0)
+			error = error1;
+	}
+	if (error == 0) {
+		error = copyout(&ccb->ccb_h + 1, u_ccbh + 1, ccb_len);
+		if (error != 0) {
+			xpt_print(softc->path,
+			    "targreturnccb - CCB copyout failed (%d)\n", error);
+		}
 	}
 	/* Free CCB or send back to devq. */
 	targfreeccb(softc, ccb);