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)