linux-kernel - Re: [PATCH] scsi: sr: fix oob access in get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <yq18to0hvib.fsf@oracle.com>
Date:   Mon, 20 Mar 2017 10:29:16 -0400
From:   "Martin K. Petersen" <martin.petersen@...cle.com>
To:     Kefeng Wang <wangkefeng.wang@...wei.com>
Cc:     "Martin K. Petersen" <martin.petersen@...cle.com>,
        Jens Axboe <axboe@...nel.dk>,
        "James E.J. Bottomley" <jejb@...ux.vnet.ibm.com>,
        <linux-scsi@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Douglas Gilbert <dgilbert@...erlog.com>
Subject: Re: [PATCH] scsi: sr: fix oob access in get_capabilities

Kefeng Wang <wangkefeng.wang@...wei.com> writes:

Kefeng,

> The issue still exists, the patch return zero in scsi_mode_sense(), but zero means
> SAM_STAT_GOOD in scsi_status_is_good(), so n will be still bigger than 512;

OK, I checked the other users of scsi_mode_sense(). So let's keep this
fix local to sr.c for now.

How about the following?


scsi: sr: Sanity check returned mode data
    
Kefeng Wang discovered that old versions of the QEMU CD driver would
return mangled mode data causing us to walk off the end of the buffer in
an attempt to parse it. Sanity check the returned mode sense data.
    
Cc: <stable@...r.kernel.org>
Reported-by: Kefeng Wang <wangkefeng.wang@...wei.com>
Signed-off-by: Martin K. Petersen <martin.petersen@...cle.com>

diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 0b29b9329b1c..a8f630213a1a 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -836,6 +836,7 @@ static void get_capabilities(struct scsi_cd *cd)
 	unsigned char *buffer;
 	struct scsi_mode_data data;
 	struct scsi_sense_hdr sshdr;
+	unsigned int ms_len = 128;
 	int rc, n;
 
 	static const char *loadmech[] =
@@ -862,10 +863,11 @@ static void get_capabilities(struct scsi_cd *cd)
 	scsi_test_unit_ready(cd->device, SR_TIMEOUT, MAX_RETRIES, &sshdr);
 
 	/* ask for mode page 0x2a */
-	rc = scsi_mode_sense(cd->device, 0, 0x2a, buffer, 128,
+	rc = scsi_mode_sense(cd->device, 0, 0x2a, buffer, ms_len,
 			     SR_TIMEOUT, 3, &data, NULL);
 
-	if (!scsi_status_is_good(rc)) {
+	if (!scsi_status_is_good(rc) || data.length > ms_len ||
+	    data.header_length + data.block_descriptor_length > data.length) {
 		/* failed, drive doesn't have capabilities mode page */
 		cd->cdi.speed = 1;
 		cd->cdi.mask |= (CDC_CD_R | CDC_CD_RW | CDC_DVD_R |