Fix privilege escalation in cd(4) driver.

Approved by:	so
Approved by:	re (implicit)
Security:	FreeBSD-SA-19:11.cd_ioctl
Security:	CVE-2019-5602

5 files changed, 26 insertions, 45 deletions

diff --git a/sys/cam/scsi/scsi_cd.c b/sys/cam/scsi/scsi_cd.c
index e9f22a1..f77924e 100644
--- a/sys/cam/scsi/scsi_cd.c
+++ b/sys/cam/scsi/scsi_cd.c
@@ -1281,7 +1281,7 @@ cdioctl(struct disk *dp, u_long cmd, void *addr, int flag, struct thread *td)
 
 	struct 	cam_periph *periph;
 	struct	cd_softc *softc;
-	int	nocopyout, error = 0;
+	int	error = 0;
 
 	periph = (struct cam_periph *)dp->d_drv1;
 	cam_periph_lock(periph);
@@ -1323,7 +1323,6 @@ cdioctl(struct disk *dp, u_long cmd, void *addr, int flag, struct thread *td)
 	 */
 	cam_periph_unlock(periph);
 
-	nocopyout = 0;
 	switch (cmd) {
 
 	case CDIOCPLAYTRACKS:
@@ -1499,9 +1498,6 @@ cdioctl(struct disk *dp, u_long cmd, void *addr, int flag, struct thread *td)
 			cam_periph_unlock(periph);
 		}
 		break;
-	case CDIOCREADSUBCHANNEL_SYSSPACE:
-		nocopyout = 1;
-		/* Fallthrough */
 	case CDIOCREADSUBCHANNEL:
 		{
 			struct ioc_read_subchannel *args
@@ -1546,13 +1542,7 @@ cdioctl(struct disk *dp, u_long cmd, void *addr, int flag, struct thread *td)
 				data->header.data_len[1] +
 				sizeof(struct cd_sub_channel_header)));
 			cam_periph_unlock(periph);
-			if (nocopyout == 0) {
-				if (copyout(data, args->data, len) != 0) {
-					error = EFAULT;
-				}
-			} else {
-				bcopy(data, args->data, len);
-			}
+			error = copyout(data, args->data, len);
 			free(data, M_SCSICD);
 		}
 		break;
diff --git a/sys/compat/linux/linux_ioctl.c b/sys/compat/linux/linux_ioctl.c
index 7a14569..a7be854 100644
--- a/sys/compat/linux/linux_ioctl.c
+++ b/sys/compat/linux/linux_ioctl.c
@@ -1546,16 +1546,26 @@ linux_ioctl_cdrom(struct thread *td, struct linux_ioctl_args *args)
 		struct ioc_read_subchannel bsdsc;
 		struct cd_sub_channel_info bsdinfo;
 
+		error = copyin((void *)args->arg, &sc, sizeof(sc));
+		if (error)
+			break;
+
+		/*
+		 * Invoke the native ioctl and bounce the returned data through
+		 * the userspace buffer.  This works because the Linux structure
+		 * is the same size as our structures for the subchannel header
+		 * and position data.
+		 */
 		bsdsc.address_format = CD_LBA_FORMAT;
 		bsdsc.data_format = CD_CURRENT_POSITION;
 		bsdsc.track = 0;
-		bsdsc.data_len = sizeof(bsdinfo);
-		bsdsc.data = &bsdinfo;
-		error = fo_ioctl(fp, CDIOCREADSUBCHANNEL_SYSSPACE,
-		    (caddr_t)&bsdsc, td->td_ucred, td);
+		bsdsc.data_len = sizeof(sc);
+		bsdsc.data = (void *)args->arg;
+		error = fo_ioctl(fp, CDIOCREADSUBCHANNEL, (caddr_t)&bsdsc,
+		    td->td_ucred, td);
 		if (error)
 			break;
-		error = copyin((void *)args->arg, &sc, sizeof(sc));
+		error = copyin((void *)args->arg, &bsdinfo, sizeof(bsdinfo));
 		if (error)
 			break;
 		sc.cdsc_audiostatus = bsdinfo.header.audio_status;
diff --git a/sys/dev/mcd/mcd.c b/sys/dev/mcd/mcd.c
index 2ce41bb..5e740ad 100644
--- a/sys/dev/mcd/mcd.c
+++ b/sys/dev/mcd/mcd.c
@@ -134,8 +134,7 @@ static void	mcd_soft_reset(struct mcd_softc *);
 static int	mcd_hard_reset(struct mcd_softc *);
 static int 	mcd_setmode(struct mcd_softc *, int mode);
 static int	mcd_getqchan(struct mcd_softc *, struct mcd_qchninfo *q);
-static int	mcd_subchan(struct mcd_softc *, struct ioc_read_subchannel *sc,
-		    int nocopyout);
+static int	mcd_subchan(struct mcd_softc *, struct ioc_read_subchannel *sc);
 static int	mcd_toc_header(struct mcd_softc *, struct ioc_toc_header *th);
 static int	mcd_read_toc(struct mcd_softc *);
 static int	mcd_toc_entrys(struct mcd_softc *, struct ioc_read_toc_entry *te);
@@ -482,10 +481,8 @@ MCD_TRACE("ioctl called 0x%lx\n", cmd);
 	case CDIOCPLAYMSF:
 		r = mcd_playmsf(sc, (struct ioc_play_msf *) addr);
 		break;
-	case CDIOCREADSUBCHANNEL_SYSSPACE:
-		return mcd_subchan(sc, (struct ioc_read_subchannel *) addr, 1);
 	case CDIOCREADSUBCHANNEL:
-		return mcd_subchan(sc, (struct ioc_read_subchannel *) addr, 0);
+		return mcd_subchan(sc, (struct ioc_read_subchannel *) addr);
 	case CDIOREADTOCHEADER:
 		r = mcd_toc_header(sc, (struct ioc_toc_header *) addr);
 		break;
@@ -1411,7 +1408,7 @@ mcd_getqchan(struct mcd_softc *sc, struct mcd_qchninfo *q)
 }
 
 static int
-mcd_subchan(struct mcd_softc *sc, struct ioc_read_subchannel *sch, int nocopyout)
+mcd_subchan(struct mcd_softc *sc, struct ioc_read_subchannel *sch)
 {
 	struct mcd_qchninfo q;
 	struct cd_sub_channel_info data;
@@ -1478,10 +1475,7 @@ mcd_subchan(struct mcd_softc *sc, struct ioc_read_subchannel *sch, int nocopyout
 	}
 
 	MCD_UNLOCK(sc);
-	if (nocopyout == 0)
-		return copyout(&data, sch->data, min(sizeof(struct cd_sub_channel_info), sch->data_len));
-	bcopy(&data, sch->data, min(sizeof(struct cd_sub_channel_info), sch->data_len));
-	return (0);
+	return (copyout(&data, sch->data, min(sizeof(struct cd_sub_channel_info), sch->data_len)));
 }
 
 static int
diff --git a/sys/dev/scd/scd.c b/sys/dev/scd/scd.c
index 1124bff..15be673 100644
--- a/sys/dev/scd/scd.c
+++ b/sys/dev/scd/scd.c
@@ -130,7 +130,7 @@ static int scd_resume(struct scd_softc *);
 static int scd_playtracks(struct scd_softc *, struct ioc_play_track *pt);
 static int scd_playmsf(struct scd_softc *, struct ioc_play_msf *msf);
 static int scd_play(struct scd_softc *, struct ioc_play_msf *msf);
-static int scd_subchan(struct scd_softc *, struct ioc_read_subchannel *sch, int nocopyout);
+static int scd_subchan(struct scd_softc *, struct ioc_read_subchannel *sch);
 static int read_subcode(struct scd_softc *, struct sony_subchannel_position_data *sch);
 
 /* for xcdplayer */
@@ -357,10 +357,8 @@ scdioctl(struct cdev *dev, u_long cmd, caddr_t addr, int flags, struct thread *t
 	case CDIOCPLAYMSF:
 		error = scd_playmsf(sc, (struct ioc_play_msf *) addr);
 		break;
-	case CDIOCREADSUBCHANNEL_SYSSPACE:
-		return scd_subchan(sc, (struct ioc_read_subchannel *) addr, 1);
 	case CDIOCREADSUBCHANNEL:
-		return scd_subchan(sc, (struct ioc_read_subchannel *) addr, 0);
+		return scd_subchan(sc, (struct ioc_read_subchannel *) addr);
 	case CDIOREADTOCHEADER:
 		error = scd_toc_header (sc, (struct ioc_toc_header *) addr);
 		break;
@@ -564,7 +562,7 @@ scd_eject(struct scd_softc *sc)
 }
 
 static int
-scd_subchan(struct scd_softc *sc, struct ioc_read_subchannel *sch, int nocopyout)
+scd_subchan(struct scd_softc *sc, struct ioc_read_subchannel *sch)
 {
 	struct sony_subchannel_position_data q;
 	struct cd_sub_channel_info data;
@@ -594,12 +592,8 @@ scd_subchan(struct scd_softc *sc, struct ioc_read_subchannel *sch, int nocopyout
 	data.what.position.absaddr.msf.frame = bcd2bin(q.abs_msf[2]);
 	SCD_UNLOCK(sc);
 
-	if (nocopyout == 0) {
-		if (copyout(&data, sch->data, min(sizeof(struct cd_sub_channel_info), sch->data_len))!=0)
-			return (EFAULT);
-	} else {
-		bcopy(&data, sch->data, min(sizeof(struct cd_sub_channel_info), sch->data_len));
-	}
+	if (copyout(&data, sch->data, min(sizeof(struct cd_sub_channel_info), sch->data_len))!=0)
+		return (EFAULT);
 	return (0);
 }
 
diff --git a/sys/sys/cdio.h b/sys/sys/cdio.h
index 3614d19..a3eb271 100644
--- a/sys/sys/cdio.h
+++ b/sys/sys/cdio.h
@@ -274,11 +274,4 @@ struct ioc_capability {			/*<2>*/
 
 #define	CDIOCCAPABILITY	_IOR('c',30,struct ioc_capability)	/*<2>*/
 
-/*
- * Special version of CDIOCREADSUBCHANNEL which assumes that
- * ioc_read_subchannel->data points to the kernel memory. For
- * use in compatibility layers.
- */
-#define CDIOCREADSUBCHANNEL_SYSSPACE _IOWR('c', 31, struct ioc_read_subchannel)
-
 #endif /* !_SYS_CDIO_H_ */