All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <dave@linux.vnet.ibm.com>
To: Matthew Wilcox <matthew@wil.cx>
Cc: linux-kernel <linux-kernel@vger.kernel.org>,
	mdharm-usb@one-eyed-alien.net,
	linux-usb <linux-usb@vger.kernel.org>,
	usb-storage@lists.one-eyed-alien.net,
	James Bottomley <James.Bottomley@HansenPartnership.com>,
	linux-scsi <linux-scsi@vger.kernel.org>,
	viro <viro@zeniv.linux.org.uk>
Subject: [PATCH v2] fix sign extension with 1.5TB usb-storage LBD=y
Date: Tue, 21 Apr 2009 15:00:10 -0700	[thread overview]
Message-ID: <1240351210.10627.30.camel@nimitz> (raw)
In-Reply-To: <20090421211858.GA1926@parisc-linux.org>

Here's a patch implementing Al's suggestion.  Not quite as trivial as
Matthew's, but even nicer aesthetically.  (Description stolen from
Matthew's patch).  I have verified that this fixes my issue.

--

Shifting an unsigned char implicitly casts it to a signed int.  This
caused 'lba' to sign-extend and Linux would then try READ CAPACITY 16
which was not supported by at least one drive.  Using the
get_unaligned_be*() helpers keeps us from having to worry about how the
extension might occur.

Signed-off-by: Dave Hansen <dave@linux.vnet.ibm.com>

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 3fcb64b..c50142b 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -50,6 +50,7 @@
 #include <linux/string_helpers.h>
 #include <linux/async.h>
 #include <asm/uaccess.h>
+#include <asm/unaligned.h>
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
@@ -1344,12 +1345,8 @@ static int read_capacity_16(struct scsi_disk *sdkp, struct scsi_device *sdp,
 		return -EINVAL;
 	}
 
-	sector_size =	(buffer[8] << 24) | (buffer[9] << 16) |
-			(buffer[10] << 8) | buffer[11];
-	lba =  (((u64)buffer[0] << 56) | ((u64)buffer[1] << 48) |
-		((u64)buffer[2] << 40) | ((u64)buffer[3] << 32) |
-		((u64)buffer[4] << 24) | ((u64)buffer[5] << 16) |
-		((u64)buffer[6] << 8) | (u64)buffer[7]);
+	sector_size = get_unaligned_be32(&buffer[8]);
+	lba =  get_unaligned_be64(&buffer[0]);
 
 	sd_read_protection_type(sdkp, buffer);
 
@@ -1400,10 +1397,8 @@ static int read_capacity_10(struct scsi_disk *sdkp, struct scsi_device *sdp,
 		return -EINVAL;
 	}
 
-	sector_size =	(buffer[4] << 24) | (buffer[5] << 16) |
-			(buffer[6] << 8) | buffer[7];
-	lba =	(buffer[0] << 24) | (buffer[1] << 16) |
-		(buffer[2] << 8) | buffer[3];
+	sector_size = get_unaligned_be32(&buffer[4]);
+	lba = get_unaligned_be32(&buffer[0]);
 
 	if ((sizeof(sdkp->capacity) == 4) && (lba == 0xffffffff)) {
 		sd_printk(KERN_ERR, sdkp, "Too big for this kernel. Use a "


-- Dave

  reply	other threads:[~2009-04-21 22:00 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-21 20:52 [RFC][PATCH] fix sign extension with 1.5TB usb-storage LBD=y Dave Hansen
2009-04-21 21:01 ` Al Viro
2009-04-21 21:01   ` Al Viro
2009-04-21 21:18 ` Matthew Wilcox
2009-04-21 22:00   ` Dave Hansen [this message]
2009-04-21 23:03     ` [PATCH v2] " Matthew Wilcox
2009-04-21 23:03       ` Matthew Wilcox
2009-04-21 23:43       ` Dave Hansen
2009-04-22  7:32     ` Boaz Harrosh
2009-04-22  7:32       ` Boaz Harrosh
     [not found]       ` <49EEC82B.5040603-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>
2009-04-22 11:09         ` Matthew Wilcox
2009-04-22 11:09           ` Matthew Wilcox
2009-04-22 11:27           ` Boaz Harrosh
     [not found]   ` <20090421211858.GA1926-6jwH94ZQLHl74goWV3ctuw@public.gmane.org>
2009-04-21 21:29     ` [RFC][PATCH] " Randy Dunlap
2009-04-21 21:29       ` Randy Dunlap
2009-04-21 21:31       ` Matthew Wilcox
2009-04-21 22:49     ` Dave Hansen
2009-04-21 22:49       ` Dave Hansen

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=1240351210.10627.30.camel@nimitz \
    --to=dave@linux.vnet.ibm.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=matthew@wil.cx \
    --cc=mdharm-usb@one-eyed-alien.net \
    --cc=usb-storage@lists.one-eyed-alien.net \
    --cc=viro@zeniv.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.