From nobody Wed Nov 03 19:11:42 2021 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 202341832A9F; Wed, 3 Nov 2021 19:11:43 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4HkxG70SMxz4lBs; Wed, 3 Nov 2021 19:11:43 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id E3C7D1E2EF; Wed, 3 Nov 2021 19:11:42 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 1A3JBgZq033169; Wed, 3 Nov 2021 19:11:42 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1A3JBgYC033168; Wed, 3 Nov 2021 19:11:42 GMT (envelope-from git) Date: Wed, 3 Nov 2021 19:11:42 GMT Message-Id: <202111031911.1A3JBgYC033168@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mark Johnston Subject: git: 6afabf00920f - main - scsi_cd: Improve TOC access validation List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 6afabf00920fb8d41b8f013090f282c17c117efc Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=6afabf00920fb8d41b8f013090f282c17c117efc commit 6afabf00920fb8d41b8f013090f282c17c117efc Author: Mark Johnston AuthorDate: 2021-11-03 19:09:17 +0000 Commit: Mark Johnston CommitDate: 2021-11-03 19:09:17 +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 [1] Reviewed by: ken, avg MFC after: 1 week Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D32803 --- 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 3e8187544bff..3cca4bbf243b 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 { @@ -1596,12 +1600,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)); /* @@ -1838,23 +1843,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;