Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 10 Nov 2021 14:24:27 GMT
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: f607b686f9a9 - stable/12 - scsi_cd: Improve TOC access validation
Message-ID:  <202111101424.1AAEOR01026339@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/12 has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=f607b686f9a96745e337a8d045f0f51b599a245e

commit f607b686f9a96745e337a8d045f0f51b599a245e
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2021-11-03 19:09:17 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2021-11-10 14:24:06 +0000

    scsi_cd: Improve TOC access validation
    
    1. During CD probing, we read the TOC header to find the number of
       entries, then read the TOC itself.  The header determines the number
       of entries, which determines the amount of data to read from the
       device into the softc in the CD_STATE_MEDIA_TOC_FULL state.  We
       hard-code a limit of 99 tracks (plus one for the lead-out) in the
       softc, but were not validating that the size reported by the media
       would fit in this hard-coded limit.  Kernel memory corruption could
       occur if not.[1]  Add validation to check this, and refuse to cache
       the TOC if it would not fit.
    
    2. The CDIOCPLAYTRACKS ioctl uses caller provided track numbers to index
       into the TOC, but we only validate the starting index.  Add
       validation of the ending index.
    
    Also, raise the hard-coded limit from 100 tracks to 170, per a
    suggestion from Ken.
    
    Reported by:    C Turt <ecturt@gmail.com> [1]
    Reviewed by:    ken, avg
    Sponsored by:   The FreeBSD Foundation
    
    (cherry picked from commit 6afabf00920fb8d41b8f013090f282c17c117efc)
---
 sys/cam/scsi/scsi_cd.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/sys/cam/scsi/scsi_cd.c b/sys/cam/scsi/scsi_cd.c
index 7f22befd3520..4f7cac43a041 100644
--- a/sys/cam/scsi/scsi_cd.c
+++ b/sys/cam/scsi/scsi_cd.c
@@ -136,9 +136,13 @@ typedef enum {
 #define ccb_state ppriv_field0
 #define ccb_bp ppriv_ptr1
 
+/*
+ * According to the MMC-6 spec, 6.25.3.2.11, the lead-out is reported by
+ * READ_TOC as logical track 170, so at most 169 tracks may be reported.
+ */
 struct cd_tocdata {
 	struct ioc_toc_header header;
-	struct cd_toc_entry entries[100];
+	struct cd_toc_entry entries[170];
 };
 
 struct cd_toc_single {
@@ -1580,12 +1584,13 @@ cddone(struct cam_periph *periph, union ccb *done_ccb)
 		}
 
 		/* Number of TOC entries, plus leadout */
-		num_entries = (toch->ending_track - toch->starting_track) + 2;
-		cdindex = toch->starting_track + num_entries -1;
+		num_entries = toch->ending_track - toch->starting_track + 2;
+		cdindex = toch->starting_track + num_entries - 1;
 
 		if ((done_ccb->ccb_h.ccb_state & CD_CCB_TYPE_MASK) ==
 		     CD_CCB_MEDIA_TOC_HDR) {
-			if (num_entries <= 0) {
+			if (num_entries <= 0 ||
+			    num_entries > nitems(softc->toc.entries)) {
 				softc->flags &= ~CD_FLAG_VALID_TOC;
 				bzero(&softc->toc, sizeof(softc->toc));
 				/*
@@ -1823,23 +1828,19 @@ cdioctl(struct disk *dp, u_long cmd, void *addr, int flag, struct thread *td)
 			 */
 			if (softc->flags & CD_FLAG_VALID_TOC) {
 				union msf_lba *sentry, *eentry;
+				struct ioc_toc_header *th;
 				int st, et;
 
-				if (args->end_track <
-				    softc->toc.header.ending_track + 1)
+				th = &softc->toc.header;
+				if (args->end_track < th->ending_track + 1)
 					args->end_track++;
-				if (args->end_track >
-				    softc->toc.header.ending_track + 1)
-					args->end_track =
-					    softc->toc.header.ending_track + 1;
-				st = args->start_track -
-					softc->toc.header.starting_track;
-				et = args->end_track -
-					softc->toc.header.starting_track;
-				if ((st < 0)
-				 || (et < 0)
-				 || (st > (softc->toc.header.ending_track -
-				     softc->toc.header.starting_track))) {
+				if (args->end_track > th->ending_track + 1)
+					args->end_track = th->ending_track + 1;
+				st = args->start_track - th->starting_track;
+				et = args->end_track - th->starting_track;
+				if (st < 0 || et < 0 ||
+				    st > th->ending_track - th->starting_track ||
+				    et > th->ending_track - th->starting_track) {
 					error = EINVAL;
 					cam_periph_unlock(periph);
 					break;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202111101424.1AAEOR01026339>