Re: [stable] [PATCH 1/2] sd: fix memory corruption by sd_read_cache_type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux