From: Stefan Richter <stefanr@s5r6.in-berlin.de>
To: James Bottomley <James.Bottomley@SteelEye.com>
Cc: Al Viro <viro@ftp.linux.org.uk>,
Linus Torvalds <torvalds@osdl.org>,
Chris Wright <chrisw@sous-sol.org>,
stable@kernel.org, Jody McIntyre <scjody@modernduck.com>,
linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org,
Andrew Morton <akpm@osdl.org>
Subject: Re: [stable] [PATCH 1/2] sd: fix memory corruption by sd_read_cache_type
Date: Sun, 26 Feb 2006 09:29:29 +0100 [thread overview]
Message-ID: <440166E9.8050608@s5r6.in-berlin.de> (raw)
In-Reply-To: <20060226053138.GM27946@ftp.linux.org.uk>
Al Viro wrote:
> On Sat, Feb 25, 2006 at 11:14:48PM -0600, James Bottomley wrote:
...
>>The problem is that it's a change
>>to sd and a change to scsi_lib in a fairly critical routine. While I'm
>>reasonably certain the change is safe, I'd prefer to make sure by
>>incubating in -mm for a while.
>>
>>The title, by the way, is misleading; it's not a memory corruption in sd
>>at all really. It's the initio bridge which produces a totally
>>standards non conformant return to a mode sense which produces the
>>problem. And so, it's only the single initio bridge which is currently
>>affected; hence the caution.
>
> 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.
>
> 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.
That's why I split Al's patch. Part 1/2 prevents sd from using values
from buggy firmwares to overwrite memory where sd has no business to
write at. As Al explained, the culprit is sd which feeds a too big len
to scsi_mode_sense()'s memset(buffer, 0, len). Patch 2/2 adds an Initio
specific workaround. (The 2nd part is not necessarily material for
-stable, although it assures correct cache handling for Initio based
SBP-2 HDDs.)
Please merge the sd fix ASAP (part 1/2). Users _are_ seeing memory
corruption or panic in interrupt context without this patch. Fully
reproducable, probably with all Initio SBP-2 bridges which exist; and
these are actually popular chips for 1394a S400 as well as 1394b S800
products, both noname and branded products.
The 2nd part could perhaps go through -mm. If you wish I resend it to
Andrew. Al, what do you think? I believe this patch to be safe for
non-broken devices though.
BTW I missed a small whitespace mishap in pt 2/2 which is why I should
repost it anyway:
-+ } else if(use_10_for_ms) {
++ } else if (use_10_for_ms) {
--
Stefan Richter
-=====-=-==- --=- ==-=-
http://arcgraph.de/sr/
next prev parent reply other threads:[~2006-02-26 8:31 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 [this message]
2006-02-26 14:34 ` James Bottomley
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=440166E9.8050608@s5r6.in-berlin.de \
--to=stefanr@s5r6.in-berlin.de \
--cc=James.Bottomley@SteelEye.com \
--cc=akpm@osdl.org \
--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=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).