⬢ DragonFlyBSD Kernel Audit
DF-5403 / fix.diff
← back to finding ↓ download raw
--- a/sys/bus/cam/scsi/scsi_sg.c
+++ b/sys/bus/cam/scsi/scsi_sg.c
@@ -443,12 +443,16 @@
 sgioctl(struct dev_ioctl_args *ap)
 /* cdev_t dev, u_long cmd, caddr_t arg, int flag, struct thread *td) */
 {
+	caddr_t addr;
 	union ccb *ccb;
 	struct ccb_scsiio *csio;
 	struct cam_periph *periph;
 	struct sg_softc *softc;
 	struct sg_io_hdr req;
-	int dir, error;
+	int dir = CAM_DIR_NONE, error;
+
+	/* DFBSD passes IOC_VOID ioctl args by value through *a_data */
+	addr = ap->a_data ? *(caddr_t *)ap->a_data : NULL;
 
 	periph = (struct cam_periph *)ap->a_head.a_dev->si_drv1;
 	if (periph == NULL)
@@ -480,12 +484,12 @@
 	}
 #endif
 	case SG_GET_VERSION_NUM:
-		error = copyout(&sg_version, ap->a_data, sizeof(sg_version));
+		error = copyout(&sg_version, addr, sizeof(sg_version));
 		break;
 	case SG_SET_TIMEOUT: {
 		u_int user_timeout;
 
-		error = copyin(ap->a_data, &user_timeout, sizeof(u_int));
+		error = copyin(addr, &user_timeout, sizeof(u_int));
 		if (error == 0) {
 			softc->sg_user_timeout = user_timeout;
 			softc->sg_timeout = user_timeout / SG_DEFAULT_HZ * hz;
@@ -500,7 +504,7 @@
 		error = 0;
 		break;
 	case SG_IO:
-		error = copyin(ap->a_data, &req, sizeof(req));
+		error = copyin(addr, &req, sizeof(req));
 		if (error)
 			break;
 
@@ -540,6 +544,18 @@
 			break;
 		}
 
+		/*
+		 * Reject the inconsistent NULL-data / non-zero-length
+		 * combination: sgsendccb() only maps when data_ptr != NULL
+		 * and the SIMs (atapi-cam.c INQUIRY, ahci_cam.c INQUIRY)
+		 * deref or DMA via csio->data_ptr without checking it.
+		 */
+		if (dir != CAM_DIR_NONE && req.dxferp == NULL &&
+		    req.dxfer_len != 0) {
+			error = EINVAL;
+			break;
+		}
+
 		cam_fill_csio(csio,
 			      /*retries*/1,
 			      sgdone,
@@ -566,7 +582,7 @@
 		req.duration = csio->ccb_h.timeout;
 		req.info = 0;
 
-		error = copyout(&req, ap->a_data, sizeof(req));
+		error = copyout(&req, addr, sizeof(req));
 		if ((error == 0) && (csio->ccb_h.status & CAM_AUTOSNS_VALID)
 		    && (req.sbp != NULL)) {
 			req.sb_len_wr = req.mx_sb_len - csio->sense_resid;
@@ -580,7 +596,7 @@
 	case SG_GET_RESERVED_SIZE: {
 		int size = 32768;
 
-		error = copyout(&size, ap->a_data, sizeof(size));
+		error = copyout(&size, addr, sizeof(size));
 		break;
 	}
 
@@ -598,7 +614,7 @@
 		id.unused[0] = 0;
 		id.unused[1] = 0;
 
-		error = copyout(&id, ap->a_data, sizeof(id));
+		error = copyout(&id, addr, sizeof(id));
 		break;
 	}