From: James Bottomley <James.Bottomley@SteelEye.com>
To: Al Viro <viro@ftp.linux.org.uk>
Cc: Linus Torvalds <torvalds@osdl.org>,
Stefan Richter <stefanr@s5r6.in-berlin.de>,
Chris Wright <chrisw@sous-sol.org>,
stable@kernel.org, Jody McIntyre <scjody@modernduck.com>,
linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org
Subject: Re: [stable] [PATCH 1/2] sd: fix memory corruption by sd_read_cache_type
Date: Sun, 26 Feb 2006 08:34:10 -0600 [thread overview]
Message-ID: <1140964451.3337.5.camel@mulgrave.il.steeleye.com> (raw)
In-Reply-To: <20060226053138.GM27946@ftp.linux.org.uk>
On Sun, 2006-02-26 at 05:31 +0000, Al Viro wrote:
> No. It's sd.c assuming that mode page header is sane, without any
> checks. And yes, it does give memory corruption if it's not.
Well, OK, I agree allowing us to request data longer than the actual
buffer is a problem. However, I don't exactly see how this actually
causes corruption, since even the initio bridge only sends 12 bytes of
data, so we should stop with a data underrun at that point (however big
the buffer is)
> Initio-related part is in scsi_lib.c (and in recovery part of sd.c
> changes). That one is about how we can handle gracefully a broken
> device that gives no header at all.
>
> Checks for ->block_descriptors_length are just making sure we won't try
> to do any access past the end of buffer, no matter what crap we got from
> device.
I agree; how about this for the actual patch (for 2.6.16) ... I put two
more tidy ups into it ...
James
---
[SCSI] sd: Add sanity checking to mode sense return data
From: Al Viro <viro@ftp.linux.org.uk>
There's a problem in sd where we blindly believe the length of the
headers and block descriptors. Some devices return insane values for
these and cause our length to end up greater than the actual buffer
size, so check to make sure.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Also removed the buffer size magic number (512) and added DPOFUA of
zero to the defaults
Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>
Index: linux-2.6/drivers/scsi/sd.c
===================================================================
--- linux-2.6.orig/drivers/scsi/sd.c
+++ linux-2.6/drivers/scsi/sd.c
@@ -89,6 +89,11 @@
#define SD_MAX_RETRIES 5
#define SD_PASSTHROUGH_RETRIES 1
+/*
+ * Size of the initial data buffer for mode and read capacity data
+ */
+#define SD_BUF_SIZE 512
+
static void scsi_disk_release(struct kref *kref);
struct scsi_disk {
@@ -1239,7 +1244,7 @@ sd_do_mode_sense(struct scsi_device *sdp
/*
* read write protect setting, if possible - called only in sd_revalidate_disk()
- * called with buffer of length 512
+ * called with buffer of length SD_BUF_SIZE
*/
static void
sd_read_write_protect_flag(struct scsi_disk *sdkp, char *diskname,
@@ -1297,7 +1302,7 @@ sd_read_write_protect_flag(struct scsi_d
/*
* sd_read_cache_type - called only from sd_revalidate_disk()
- * called with buffer of length 512
+ * called with buffer of length SD_BUF_SIZE
*/
static void
sd_read_cache_type(struct scsi_disk *sdkp, char *diskname,
@@ -1342,6 +1347,8 @@ sd_read_cache_type(struct scsi_disk *sdk
/* Take headers and block descriptors into account */
len += data.header_length + data.block_descriptor_length;
+ if (len > SD_BUF_SIZE)
+ goto bad_sense;
/* Get the data */
res = sd_do_mode_sense(sdp, dbd, modepage, buffer, len, &data, &sshdr);
@@ -1354,6 +1361,12 @@ sd_read_cache_type(struct scsi_disk *sdk
int ct = 0;
int offset = data.header_length + data.block_descriptor_length;
+ if (offset >= SD_BUF_SIZE - 2) {
+ printk(KERN_ERR "%s: malformed MODE SENSE response",
+ diskname);
+ goto defaults;
+ }
+
if ((buffer[offset] & 0x3f) != modepage) {
printk(KERN_ERR "%s: got wrong page\n", diskname);
goto defaults;
@@ -1398,6 +1411,7 @@ defaults:
diskname);
sdkp->WCE = 0;
sdkp->RCD = 0;
+ sdkp->DPOFUA = 0;
}
/**
@@ -1421,7 +1435,7 @@ static int sd_revalidate_disk(struct gen
if (!scsi_device_online(sdp))
goto out;
- buffer = kmalloc(512, GFP_KERNEL | __GFP_DMA);
+ buffer = kmalloc(SD_BUF_SIZE, GFP_KERNEL | __GFP_DMA);
if (!buffer) {
printk(KERN_WARNING "(sd_revalidate_disk:) Memory allocation "
"failure.\n");
next prev parent reply other threads:[~2006-02-26 14:35 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-02-23 1:02 [PATCH 1/2] sd: fix memory corruption by sd_read_cache_type Stefan Richter
2006-02-25 2:10 ` [stable] " Chris Wright
2006-02-25 23:07 ` Stefan Richter
2006-02-25 23:22 ` Al Viro
2006-02-26 8:11 ` Stefan Richter
2006-02-26 8:22 ` Al Viro
2006-02-26 9:11 ` Stefan Richter
2006-02-26 0:01 ` Linus Torvalds
2006-02-26 0:17 ` Al Viro
2006-02-26 0:39 ` Linus Torvalds
2006-02-26 8:39 ` Jeff Garzik
2006-02-26 9:00 ` Al Viro
2006-02-26 10:45 ` Jeff Garzik
2006-02-26 11:47 ` Al Viro
2006-02-26 5:14 ` James Bottomley
2006-02-26 5:31 ` Al Viro
2006-02-26 8:29 ` Stefan Richter
2006-02-26 14:34 ` James Bottomley [this message]
2006-02-26 14:57 ` Al Viro
2006-02-26 16:21 ` Stefan Richter
2006-02-26 23:16 ` [PATCH 2.6.15.4 update] sd: fix memory corruption with broken mode page headers Stefan Richter
2006-02-27 20:25 ` [stable] " Chris Wright
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1140964451.3337.5.camel@mulgrave.il.steeleye.com \
--to=james.bottomley@steeleye.com \
--cc=chrisw@sous-sol.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=scjody@modernduck.com \
--cc=stable@kernel.org \
--cc=stefanr@s5r6.in-berlin.de \
--cc=torvalds@osdl.org \
--cc=viro@ftp.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).