DragonFlyBSD Kernel Audit
DF-0989 / fix.diff
← back to finding ↓ download raw
diff --git a/sys/bus/cam/scsi/scsi_cd.c b/sys/bus/cam/scsi/scsi_cd.c
index 0000000..1111111 100644
--- a/sys/bus/cam/scsi/scsi_cd.c
+++ b/sys/bus/cam/scsi/scsi_cd.c
@@ -4280,23 +4280,41 @@
 
 		break;
 	}
-	default:
+	default: {
 		/*
 		 * Tell the user what the overall length is, no matter
 		 * what we can actually fit in the data buffer.
+		 *
+		 * Beware underflow: a malicious or buggy device may return
+		 * short data with csio.resid >= length, in which case the
+		 * subtraction `length - resid - sizeof(header)` underflows
+		 * (size_t)-N and is later widened to a huge positive value.
+		 * That would cause the bcopy below to read past databuf
+		 * (which is only `length` bytes) and leak kernel heap to
+		 * userspace via dvdstruct->data. Clamp at zero instead.
 		 */
-		dvdstruct->length = length - ccb->csio.resid - 
-			sizeof(struct scsi_read_dvd_struct_data_header);
+		size_t hdr_sz = sizeof(struct scsi_read_dvd_struct_data_header);
+		size_t avail  = (size_t)length;
+		size_t resid  = ccb->csio.resid;
+		size_t copied;
 
-		/*
-		 * But only actually copy out the smaller of what we read
-		 * in or what the structure can take.
-		 */
+		if (avail > resid)
+			copied = avail - resid;
+		else
+			copied = 0;
+		if (copied > hdr_sz)
+			copied -= hdr_sz;
+		else
+			copied = 0;
+		if (copied > sizeof(dvdstruct->data))
+			copied = sizeof(dvdstruct->data);
+
+		dvdstruct->length = copied;
 		bcopy(databuf + sizeof(struct scsi_read_dvd_struct_data_header),
-		      dvdstruct->data,
-		      min(sizeof(dvdstruct->data), dvdstruct->length));
+		      dvdstruct->data, copied);
 		break;
 	}
+	}
 bailout:
 
 	if (databuf != NULL)