[PATCH] sd: Fix regression in sd_read_cache_type

[PATCH] sd: Fix regression in sd_read_cache_type

[Alan Stern]

This patch (as1454) fixes a regression in the sd driver.  Commit
24d720b726c1a85f1962831ac30ad4d2ef8276b1 ([SCSI] Retrieve the Caching
mode page) recently introduced the strategy of asking for all pages
(page code 0x3F) instead of asking for the caching page (0x08) on
devices that might not support it.  This ought to be safe, because sd
already uses page code 0x3F when checking for write protection.

Unfortunately, the commit did not copy the checks used by
sd_read_write_protect_flag().  Some devices don't support page code
0x3F, and others require a fixed transfer length of 192 bytes.  This
patch adds those checks into sd_read_cache_type().

Without this fix, some USB mass-storage devices crash when they
receive a MODE SENSE command with page code 0x3F asking for only 4
bytes of data.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-and-tested-by: Richard Senior <richard@r-senior.demon.co.uk>
CC: Luben Tuikov <ltuikov@yahoo.com>
CC: <stable@kernel.org>

---

The only stable kernel requiring this fix is 2.6.38; earlier kernels
are okay.


 drivers/scsi/sd.c |   14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Index: usb-2.6/drivers/scsi/sd.c
===================================================================
--- usb-2.6.orig/drivers/scsi/sd.c
+++ usb-2.6/drivers/scsi/sd.c
@@ -1904,17 +1904,23 @@ sd_read_cache_type(struct scsi_disk *sdk
 
 	int dbd;
 	int modepage;
+	int first_len;
 	struct scsi_mode_data data;
 	struct scsi_sense_hdr sshdr;
 	int old_wce = sdkp->WCE;
 	int old_rcd = sdkp->RCD;
 	int old_dpofua = sdkp->DPOFUA;
 
+	first_len = 4;
 	if (sdp->skip_ms_page_8) {
 		if (sdp->type == TYPE_RBC)
 			goto defaults;
 		else {
+			if (sdp->skip_ms_page_3f)
+				goto defaults;
 			modepage = 0x3F;
+			if (sdp->use_192_bytes_for_3f)
+				first_len = 192;
 			dbd = 0;
 		}
 	} else if (sdp->type == TYPE_RBC) {
@@ -1926,13 +1932,15 @@ sd_read_cache_type(struct scsi_disk *sdk
 	}
 
 	/* cautiously ask */
-	res = sd_do_mode_sense(sdp, dbd, modepage, buffer, 4, &data, &sshdr);
+	res = sd_do_mode_sense(sdp, dbd, modepage, buffer, first_len,
+			&data, &sshdr);
 
 	if (!scsi_status_is_good(res))
 		goto bad_sense;
 
 	if (!data.header_length) {
 		modepage = 6;
+		first_len = 0;
 		sd_printk(KERN_ERR, sdkp, "Missing header in MODE_SENSE response\n");
 	}
 
@@ -1952,7 +1960,9 @@ sd_read_cache_type(struct scsi_disk *sdk
 	}
 
 	/* Get the data */
-	res = sd_do_mode_sense(sdp, dbd, modepage, buffer, len, &data, &sshdr);
+	if (len > first_len)
+		res = sd_do_mode_sense(sdp, dbd, modepage, buffer, len,
+				&data, &sshdr);
 
 	if (scsi_status_is_good(res)) {
 		int offset = data.header_length + data.block_descriptor_length;

Re: [PATCH] sd: Fix regression in sd_read_cache_type

[Luben Tuikov]

--- On Mon, 3/21/11, Alan Stern <stern@rowland.harvard.edu> wrote:
> From: Alan Stern <stern@rowland.harvard.edu>
> Subject: [PATCH] sd: Fix regression in sd_read_cache_type
> To: "James Bottomley" <James.Bottomley@suse.de>
> Cc: "Richard Senior" <richard@r-senior.demon.co.uk>, "Luben Tuikov" <ltuikov@yahoo.com>, "SCSI development list" <linux-scsi@vger.kernel.org>
> Date: Monday, March 21, 2011, 2:29 PM
> This patch (as1454) fixes a
> regression in the sd driver.  Commit
> 24d720b726c1a85f1962831ac30ad4d2ef8276b1 ([SCSI] Retrieve
> the Caching
> mode page) recently introduced the strategy of asking for
> all pages
> (page code 0x3F) instead of asking for the caching page
> (0x08) on
> devices that might not support it.  This ought to be
> safe, because sd
> already uses page code 0x3F when checking for write
> protection.
> 
> Unfortunately, the commit did not copy the checks used by
> sd_read_write_protect_flag().  Some devices don't
> support page code
> 0x3F, and others require a fixed transfer length of 192
> bytes.  This
> patch adds those checks into sd_read_cache_type().
> 
> Without this fix, some USB mass-storage devices crash when
> they
> receive a MODE SENSE command with page code 0x3F asking for
> only 4
> bytes of data.
> 
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Reported-and-tested-by: Richard Senior <richard@r-senior.demon.co.uk>
> CC: Luben Tuikov <ltuikov@yahoo.com>
> CC: <stable@kernel.org>

Acked-by: Luben Tuikov <ltuikov@yahoo.com>

> 
> ---
> 
> The only stable kernel requiring this fix is 2.6.38;
> earlier kernels
> are okay.
> 
> 
>  drivers/scsi/sd.c |   14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> Index: usb-2.6/drivers/scsi/sd.c
> ===================================================================
> --- usb-2.6.orig/drivers/scsi/sd.c
> +++ usb-2.6/drivers/scsi/sd.c
> @@ -1904,17 +1904,23 @@ sd_read_cache_type(struct scsi_disk
> *sdk
>  
>      int dbd;
>      int modepage;
> +    int first_len;
>      struct scsi_mode_data data;
>      struct scsi_sense_hdr sshdr;
>      int old_wce = sdkp->WCE;
>      int old_rcd = sdkp->RCD;
>      int old_dpofua = sdkp->DPOFUA;
>  
> +    first_len = 4;
>      if (sdp->skip_ms_page_8) {
>          if (sdp->type ==
> TYPE_RBC)
>             
> goto defaults;
>          else {
> +           
> if (sdp->skip_ms_page_3f)
> +           
>     goto defaults;
>             
> modepage = 0x3F;
> +           
> if (sdp->use_192_bytes_for_3f)
> +           
>     first_len = 192;
>             
> dbd = 0;
>          }
>      } else if (sdp->type == TYPE_RBC) {
> @@ -1926,13 +1932,15 @@ sd_read_cache_type(struct scsi_disk
> *sdk
>      }
>  
>      /* cautiously ask */
> -    res = sd_do_mode_sense(sdp, dbd,
> modepage, buffer, 4, &data, &sshdr);
> +    res = sd_do_mode_sense(sdp, dbd,
> modepage, buffer, first_len,
> +           
> &data, &sshdr);
>  
>      if (!scsi_status_is_good(res))
>          goto bad_sense;
>  
>      if (!data.header_length) {
>          modepage = 6;
> +        first_len = 0;
>          sd_printk(KERN_ERR,
> sdkp, "Missing header in MODE_SENSE response\n");
>      }
>  
> @@ -1952,7 +1960,9 @@ sd_read_cache_type(struct scsi_disk
> *sdk
>      }
>  
>      /* Get the data */
> -    res = sd_do_mode_sense(sdp, dbd,
> modepage, buffer, len, &data, &sshdr);
> +    if (len > first_len)
> +        res =
> sd_do_mode_sense(sdp, dbd, modepage, buffer, len,
> +           
>     &data, &sshdr);
>  
>      if (scsi_status_is_good(res)) {
>          int offset =
> data.header_length + data.block_descriptor_length;
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sd: Fix regression in sd_read_cache_type

[James Bottomley]

On Wed, 2011-03-23 at 07:06 -0700, Luben Tuikov wrote:
> --- On Mon, 3/21/11, Alan Stern <stern@rowland.harvard.edu> wrote:
> > From: Alan Stern <stern@rowland.harvard.edu>
> > Subject: [PATCH] sd: Fix regression in sd_read_cache_type
> > To: "James Bottomley" <James.Bottomley@suse.de>
> > Cc: "Richard Senior" <richard@r-senior.demon.co.uk>, "Luben Tuikov" <ltuikov@yahoo.com>, "SCSI development list" <linux-scsi@vger.kernel.org>
> > Date: Monday, March 21, 2011, 2:29 PM
> > This patch (as1454) fixes a
> > regression in the sd driver.  Commit
> > 24d720b726c1a85f1962831ac30ad4d2ef8276b1 ([SCSI] Retrieve
> > the Caching
> > mode page) recently introduced the strategy of asking for
> > all pages
> > (page code 0x3F) instead of asking for the caching page
> > (0x08) on
> > devices that might not support it.  This ought to be
> > safe, because sd
> > already uses page code 0x3F when checking for write
> > protection.
> > 
> > Unfortunately, the commit did not copy the checks used by
> > sd_read_write_protect_flag().  Some devices don't
> > support page code
> > 0x3F, and others require a fixed transfer length of 192
> > bytes.  This
> > patch adds those checks into sd_read_cache_type().
> > 
> > Without this fix, some USB mass-storage devices crash when
> > they
> > receive a MODE SENSE command with page code 0x3F asking for
> > only 4
> > bytes of data.
> > 
> > Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> > Reported-and-tested-by: Richard Senior <richard@r-senior.demon.co.uk>
> > CC: Luben Tuikov <ltuikov@yahoo.com>
> > CC: <stable@kernel.org>
> 
> Acked-by: Luben Tuikov <ltuikov@yahoo.com>

I put the original patch in on the understanding from both of you that
the chances of finding a USB device which crashed with the change was
very small.

Given that several have been found and we're on the eve of the merge
window closure, I'll just revert the original, and you two can work on
getting a bullet proof version for the next merge window.

James

Re: [PATCH] sd: Fix regression in sd_read_cache_type

[Luben Tuikov]

--- On Wed, 3/23/11, James Bottomley <James.Bottomley@suse.de> wrote:
> 
> I put the original patch in on the understanding from both
> of you that
> the chances of finding a USB device which crashed with the
> change was
> very small.

Bottomley, obviously there are no guarantees in life. This is what bug fixes and code development is all about. It's all about making things better for for a wider range of NEW devices.

> Given that several have been found and we're on the eve of
> the merge window closure,

It wouldn't have mattered it if was found two weeks ago or in a month.

> I'll just revert the original,

Obviously you don't have to do that if you applied Alan patch.

> and you two can work on getting a bullet proof version for the next
> merge window.

Here is what happened: I submitted a patch and it was generally liked. To
refresh your memory here is the thread: http://marc.info/?t=129044508400003&r=1&w=2. The patch was applied. Then an odd device was
found, as reported by Alan, who fixed it, and submitted a patch. He agreed
that his patch works on top of mine to fix the problem. It was verified to
fix the problem by Richard. Then I agreed with Alan that his patch works
on top of mine to fix the problem by acking it as you can see in this
thread. So Alan and I have both agreed that this is, at the moment, a
"bullet-proof" version for this and any other merge window.

Having said that, you should be aware that no patch is "bullet-proof",
neither is the SCSI Layer, nor is the Linux kernel. That is, as technology
goes forward.

If you prefer to revert the original patch, I can resubmit the original
patch with Alan's patch on top of it (as it is a fix for the original,
larger, topic patch), and then you can apply that.

Let me know what would work for you Bottomley.

    Luben

Re: [PATCH] sd: Fix regression in sd_read_cache_type

[Alan Stern]

On Wed, 23 Mar 2011, James Bottomley wrote:

> > > Without this fix, some USB mass-storage devices crash when
> > > they
> > > receive a MODE SENSE command with page code 0x3F asking for
> > > only 4
> > > bytes of data.
> > > 
> > > Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> > > Reported-and-tested-by: Richard Senior <richard@r-senior.demon.co.uk>
> > > CC: Luben Tuikov <ltuikov@yahoo.com>
> > > CC: <stable@kernel.org>
> > 
> > Acked-by: Luben Tuikov <ltuikov@yahoo.com>
> 
> I put the original patch in on the understanding from both of you that
> the chances of finding a USB device which crashed with the change was
> very small.

I should have checked the patch against the context of the existing 
code more carefully.  :-(

> Given that several have been found and we're on the eve of the merge
> window closure, I'll just revert the original, and you two can work on
> getting a bullet proof version for the next merge window.

Reverting the original patch for now is fine with me.  As for the next
merge window, let me submit a more bullet-proof version of the second
patch.

It's possible that some wierd USB device will report that more than 192
bytes of mode-sense data is available and then fail when the host asks
for the reported amount.  You'd think no device could be that stupid,
but I have seen an example of a device doing exactly this (except that
it was for INQUIRY data rather than MODE SENSE data -- which is perhaps
even worse!).

Alan Stern

Re: [PATCH] sd: Fix regression in sd_read_cache_type

[Luben Tuikov]

--- On Wed, 3/23/11, Alan Stern <stern@rowland.harvard.edu> wrote:
> 
> Reverting the original patch for now is fine with me. 

You mean my patch? This commit: 24d720b726c1a85f1962831ac30ad4d2ef8276b1.

> As for the next
> merge window, let me submit a more bullet-proof version of
> the second
> patch.

How can you submit a "more bullet-proof" version of the second patch without the "first", which would be my topic patch I assume? This
one: 24d720b726c1a85f1962831ac30ad4d2ef8276b1.

Does this mean you'll resubmit my topic patch as your own and roll your minor patch with it?

> It's possible that some wierd USB device will report that
> more than 192
> bytes of mode-sense data is available and then fail when
> the host asks
> for the reported amount.  You'd think no device could
> be that stupid,

If by "fail" you mean "crash", then yes, that's bad. However, the device may return less data. The while loop of my patch, 24d720b726c1a85f1962831ac30ad4d2ef8276b1, will correctly parse the returned
less data.

> but I have seen an example of a device doing exactly this
> (except that
> it was for INQUIRY data rather than MODE SENSE data --
> which is perhaps
> even worse!).

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sd: Fix regression in sd_read_cache_type

[Alan Stern]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF-8, Size: 1945 bytes --]

On Wed, 23 Mar 2011, Luben Tuikov wrote:

> --- On Wed, 3/23/11, Alan Stern <stern@rowland.harvard.edu> wrote:
> 
> > Reverting the original patch for now is fine with me. 
> 
> You mean my patch? This commit: 24d720b726c1a85f1962831ac30ad4d2ef8276b1.

Yes, that is what I mean by "the original patch".

> > As for the next
> > merge window, let me submit a more bullet-proof version of
> > the second
> > patch.
> 
> How can you submit a "more bullet-proof" version of the second patch without the "first", which would be my topic patch I assume? This
> one: 24d720b726c1a85f1962831ac30ad4d2ef8276b1.
> 
> Does this mean you'll resubmit my topic patch as your own and roll your minor patch with it?

No.  It means I'll submit a new version of the $SUBJECT patch (which in 
fact I have already done), and leave it up to James to decide the best 
way to merge it with your original patch.

Even though the merge window is now nearly over, my revised patch could
still be sent on to Linus as a bug fix for 2.6.39-rc2 (and also queued
for 2.6.38.1 or .2, of course).

> > It's possible that some wierd USB device will report that
> > more than 192
> > bytes of mode-sense data is available and then fail when
> > the host asks
> > for the reported amount.  You'd think no device could
> > be that stupid,
> 
> If by "fail" you mean "crash", then yes, that's bad. However, the device may return less data. The while loop of my patch, 24d720b726c1a85f1962831ac30ad4d2ef8276b1, will correctly parse the returned
> less data.

True; my patches catch that case.  They prevent a second MODE SENSE 
command from being sent if it would ask for less data than the first 
command has already asked for.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sd: Fix regression in sd_read_cache_type

[Luben Tuikov]

--- On Wed, 3/23/11, Alan Stern <stern@rowland.harvard.edu> wrote:
> 
> No.  It means I'll submit a new version of the
> $SUBJECT patch (which in 
> fact I have already done),

I just saw it and acked it.

> and leave it up to James to
> decide the best 
> way to merge it with your original patch.

Yes, I'm curious to see what Bottomley does too, now that he's (apparently?) decided to revert my topic patch, on top of which yours applies.

> Even though the merge window is now nearly over, my revised
> patch could
> still be sent on to Linus as a bug fix for 2.6.39-rc2 (and
> also queued
> for 2.6.38.1 or .2, of course).

Yes, that'd be the smart thing to do.

> > > It's possible that some wierd USB device will
> report that
> > > more than 192
> > > bytes of mode-sense data is available and then
> fail when
> > > the host asks
> > > for the reported amount.  You'd think no device
> could
> > > be that stupid,
> > 
> > If by "fail" you mean "crash", then yes, that's bad.
> However, the device may return less data. The while loop of
> my patch, 24d720b726c1a85f1962831ac30ad4d2ef8276b1, will
> correctly parse the returned
> > less data.
> 
> True; my patches catch that case.  They prevent a
> second MODE SENSE 
> command from being sent if it would ask for less data than
> the first 
> command has already asked for.

By "that" case you mean asking for less data than already asked for. You don't mean "parsing the MOSE SENSE data for any len correctly".

Indeed, there is no point in asking for _less_ data than already asked for, as the loop I included in my topic patch, 24d720b726c1a85f1962831ac30ad4d2ef8276b1, will parse that data correctly.

  Luben


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] sd: Fix regression in sd_read_cache_type

[Alan Stern]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: TEXT/PLAIN; charset=UTF-8, Size: 1303 bytes --]

On Wed, 23 Mar 2011, Luben Tuikov wrote:

> --- On Wed, 3/23/11, Alan Stern <stern@rowland.harvard.edu> wrote:
> 
> > No.  It means I'll submit a new version of the
> > $SUBJECT patch (which in 
> > fact I have already done),
> 
> I just saw it and acked it.

Thank you.

> > > If by "fail" you mean "crash", then yes, that's bad.
> > However, the device may return less data. The while loop of
> > my patch, 24d720b726c1a85f1962831ac30ad4d2ef8276b1, will
> > correctly parse the returned
> > > less data.
> > 
> > True; my patches catch that case.  They prevent a
> > second MODE SENSE 
> > command from being sent if it would ask for less data than
> > the first 
> > command has already asked for.
> 
> By "that" case you mean asking for less data than already asked for. You don't mean "parsing the MOSE SENSE data for any len correctly".

Correct.

> Indeed, there is no point in asking for _less_ data than already asked for, as the loop I included in my topic patch, 24d720b726c1a85f1962831ac30ad4d2ef8276b1, will parse that data correctly.

Exactly.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH ver. 2] sd: Fix regression in sd_read_cache_type

[Alan Stern]

This patch (as1454b) fixes a regression in the sd driver.  Commit
24d720b726c1a85f1962831ac30ad4d2ef8276b1 ([SCSI] Retrieve the Caching
mode page) recently introduced the strategy of asking for all pages
(page code 0x3F) instead of asking for the caching page (0x08) on
devices that might not support it.  This ought to be safe, because sd
already uses page code 0x3F when checking for write protection.

Unfortunately, the commit did not copy the checks used by
sd_read_write_protect_flag().  Some devices don't support page code
0x3F, and others require a fixed transfer length of 192 bytes.  This
patch adds those checks into sd_read_cache_type().

Without this fix, some USB mass-storage devices crash when they
receive a MODE SENSE command with page code 0x3F asking for only 4
bytes of data.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-and-tested-by: Richard Senior <richard@r-senior.demon.co.uk>
CC: Luben Tuikov <ltuikov@yahoo.com>
CC: <stable@kernel.org>

---

 drivers/scsi/sd.c |   16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Index: usb-2.6/drivers/scsi/sd.c
===================================================================
--- usb-2.6.orig/drivers/scsi/sd.c
+++ usb-2.6/drivers/scsi/sd.c
@@ -1904,17 +1904,23 @@ sd_read_cache_type(struct scsi_disk *sdk
 
 	int dbd;
 	int modepage;
+	int first_len;
 	struct scsi_mode_data data;
 	struct scsi_sense_hdr sshdr;
 	int old_wce = sdkp->WCE;
 	int old_rcd = sdkp->RCD;
 	int old_dpofua = sdkp->DPOFUA;
 
+	first_len = 4;
 	if (sdp->skip_ms_page_8) {
 		if (sdp->type == TYPE_RBC)
 			goto defaults;
 		else {
+			if (sdp->skip_ms_page_3f)
+				goto defaults;
 			modepage = 0x3F;
+			if (sdp->use_192_bytes_for_3f)
+				first_len = 192;
 			dbd = 0;
 		}
 	} else if (sdp->type == TYPE_RBC) {
@@ -1926,13 +1932,15 @@ sd_read_cache_type(struct scsi_disk *sdk
 	}
 
 	/* cautiously ask */
-	res = sd_do_mode_sense(sdp, dbd, modepage, buffer, 4, &data, &sshdr);
+	res = sd_do_mode_sense(sdp, dbd, modepage, buffer, first_len,
+			&data, &sshdr);
 
 	if (!scsi_status_is_good(res))
 		goto bad_sense;
 
 	if (!data.header_length) {
 		modepage = 6;
+		first_len = 0;
 		sd_printk(KERN_ERR, sdkp, "Missing header in MODE_SENSE response\n");
 	}
 
@@ -1950,9 +1958,13 @@ sd_read_cache_type(struct scsi_disk *sdk
 			  "data from %d to %d bytes\n", len, SD_BUF_SIZE);
 		len = SD_BUF_SIZE;
 	}
+	if (modepage == 0x3F && sdp->use_192_bytes_for_3f)
+		len = 192;
 
 	/* Get the data */
-	res = sd_do_mode_sense(sdp, dbd, modepage, buffer, len, &data, &sshdr);
+	if (len > first_len)
+		res = sd_do_mode_sense(sdp, dbd, modepage, buffer, len,
+				&data, &sshdr);
 
 	if (scsi_status_is_good(res)) {
 		int offset = data.header_length + data.block_descriptor_length;

Re: [PATCH ver. 2] sd: Fix regression in sd_read_cache_type

[Luben Tuikov]

--- On Wed, 3/23/11, Alan Stern <stern@rowland.harvard.edu> wrote:

> From: Alan Stern <stern@rowland.harvard.edu>
> Subject: [PATCH ver. 2] sd: Fix regression in sd_read_cache_type
> To: "James Bottomley" <James.Bottomley@suse.de>
> Cc: ltuikov@yahoo.com, "Richard Senior" <richard@r-senior.demon.co.uk>, "SCSI development list" <linux-scsi@vger.kernel.org>
> Date: Wednesday, March 23, 2011, 9:18 AM
> This patch (as1454b) fixes a
> regression in the sd driver.  Commit
> 24d720b726c1a85f1962831ac30ad4d2ef8276b1 ([SCSI] Retrieve
> the Caching
> mode page) recently introduced the strategy of asking for
> all pages
> (page code 0x3F) instead of asking for the caching page
> (0x08) on
> devices that might not support it.  This ought to be
> safe, because sd
> already uses page code 0x3F when checking for write
> protection.
> 
> Unfortunately, the commit did not copy the checks used by
> sd_read_write_protect_flag().  Some devices don't
> support page code
> 0x3F, and others require a fixed transfer length of 192
> bytes.  This
> patch adds those checks into sd_read_cache_type().
> 
> Without this fix, some USB mass-storage devices crash when
> they
> receive a MODE SENSE command with page code 0x3F asking for
> only 4
> bytes of data.
> 
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Reported-and-tested-by: Richard Senior <richard@r-senior.demon.co.uk>
> CC: Luben Tuikov <ltuikov@yahoo.com>

Acked-by: Luben Tuikov <ltuikov@yahoo.com>

> CC: <stable@kernel.org>
> 
> ---
> 
>  drivers/scsi/sd.c |   16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> Index: usb-2.6/drivers/scsi/sd.c
> ===================================================================
> --- usb-2.6.orig/drivers/scsi/sd.c
> +++ usb-2.6/drivers/scsi/sd.c
> @@ -1904,17 +1904,23 @@ sd_read_cache_type(struct scsi_disk
> *sdk
>  
>      int dbd;
>      int modepage;
> +    int first_len;
>      struct scsi_mode_data data;
>      struct scsi_sense_hdr sshdr;
>      int old_wce = sdkp->WCE;
>      int old_rcd = sdkp->RCD;
>      int old_dpofua = sdkp->DPOFUA;
>  
> +    first_len = 4;
>      if (sdp->skip_ms_page_8) {
>          if (sdp->type ==
> TYPE_RBC)
>             
> goto defaults;
>          else {
> +           
> if (sdp->skip_ms_page_3f)
> +           
>     goto defaults;
>             
> modepage = 0x3F;
> +           
> if (sdp->use_192_bytes_for_3f)
> +           
>     first_len = 192;
>             
> dbd = 0;
>          }
>      } else if (sdp->type == TYPE_RBC) {
> @@ -1926,13 +1932,15 @@ sd_read_cache_type(struct scsi_disk
> *sdk
>      }
>  
>      /* cautiously ask */
> -    res = sd_do_mode_sense(sdp, dbd,
> modepage, buffer, 4, &data, &sshdr);
> +    res = sd_do_mode_sense(sdp, dbd,
> modepage, buffer, first_len,
> +           
> &data, &sshdr);
>  
>      if (!scsi_status_is_good(res))
>          goto bad_sense;
>  
>      if (!data.header_length) {
>          modepage = 6;
> +        first_len = 0;
>          sd_printk(KERN_ERR,
> sdkp, "Missing header in MODE_SENSE response\n");
>      }
>  
> @@ -1950,9 +1958,13 @@ sd_read_cache_type(struct scsi_disk
> *sdk
>             
>   "data from %d to %d bytes\n", len, SD_BUF_SIZE);
>          len = SD_BUF_SIZE;
>      }
> +    if (modepage == 0x3F &&
> sdp->use_192_bytes_for_3f)
> +        len = 192;
>  
>      /* Get the data */
> -    res = sd_do_mode_sense(sdp, dbd,
> modepage, buffer, len, &data, &sshdr);
> +    if (len > first_len)
> +        res =
> sd_do_mode_sense(sdp, dbd, modepage, buffer, len,
> +           
>     &data, &sshdr);
>  
>      if (scsi_status_is_good(res)) {
>          int offset =
> data.header_length + data.block_descriptor_length;
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html