Subject: Re: sd(4) block size detection (Re: SCSI Mode Sense on page 0)
To: Manuel Bouyer <bouyer@antioche.eu.org>
From: Rhialto <rhialto@falu.nl>
List: tech-kern
Date: 11/28/2006 23:46:42
On Fri 24 Nov 2006 at 19:58:06 +0100, Manuel Bouyer wrote:
> On Sat, Nov 25, 2006 at 02:58:07AM +0900, ITOH Yasufumi wrote:
> > I noticed the page contents of Mode Sense is not used, but
> > the block descriptor (sent prior to the page contents) is used,

Sorry to respond to this a bit late, but I have a possibly relevant
related issue with umass devices. In particular, I have one umass device
(a "Cooler Master") that seems to respond incorrectly to *any* mode
sense, whether via usb or firewire. The format it returns is so wrong
that it crashes the kernal.  To get around it, I added some sanity
checks and a quirk entry.  (I posted about this before, see the thread
beginning at
http://mail-index.netbsd.org/current-users/2006/09/08/0007.html and
ending at
http://mail-index.netbsd.org/current-users/2006/09/11/0001.html with a
detailed dump of the data that is returned, and a patch. I repeat the
patch here:


Index: scsiconf.c
===================================================================
RCS file: /cvsroot/src/sys/dev/scsipi/scsiconf.c,v
retrieving revision 1.236
diff -u -r1.236 scsiconf.c
--- scsiconf.c	30 Mar 2006 16:09:28 -0000	1.236
+++ scsiconf.c	11 Sep 2006 00:12:48 -0000
@@ -571,6 +571,8 @@
 	/* Broken IBM disk */
 	{{T_DIRECT, T_FIXED,
 	 ""	   , "DFRSS2F",		 ""},	  PQUIRK_AUTOSAVE},
+	{{T_DIRECT, T_FIXED,
+	 "Initio  ", "",		 ""},	  PQUIRK_NOBIGMODESENSE},
 	{{T_DIRECT, T_REMOV,
 	 "MPL     ", "MC-DISK-        ", ""},     PQUIRK_NOLUNS},
 	{{T_DIRECT, T_FIXED,

The range check could be made stricter (sizeof(scsipi_sense) - sizeof
(what we're looking for)) but it works for weeding out the gross
nonsense.

Index: sd.c
===================================================================
RCS file: /cvsroot/src/sys/dev/scsipi/sd.c,v
retrieving revision 1.247
diff -u -r1.247 sd.c
--- sd.c	30 Mar 2006 16:09:28 -0000	1.247
+++ sd.c	11 Sep 2006 00:12:48 -0000
@@ -1816,6 +1816,9 @@
 		poffset += scsipi_sense.header.small.blk_desc_len;
 	}
 
+	if (poffset > sizeof(scsipi_sense))
+		return ERESTART;
+
 	pages = (void *)((u_long)&scsipi_sense + poffset);
 #if 0
 printf("page 4 sense:"); for (i = sizeof(scsipi_sense), p = (void *)&scsipi_sense; i; i--, p++) printf(" %02x", *p); printf("\n");
@@ -1891,6 +1894,9 @@
 		poffset += scsipi_sense.header.small.blk_desc_len;
 	}
 
+	if (poffset > sizeof(scsipi_sense))
+		return ERESTART;
+
 	pages = (void *)((u_long)&scsipi_sense + poffset);
 #if 0
 printf("page 5 sense:"); for (i = sizeof(scsipi_sense), p = (void *)&scsipi_sense; i; i--, p++) printf(" %02x", *p); printf("\n");

These patches apply to -current with some offset.

-Olaf.
-- 
___ Olaf 'Rhialto' Seibert      -- You author it, and I'll reader it.
\X/ rhialto/at/xs4all.nl        -- Cetero censeo "authored" delendum esse.