Bug #3220

noncompliant devices break sd-config-list

Added by Keith Wesolowski about 6 years ago. Updated about 6 years ago.

Status:NewStart date:2012-09-24
Priority:NormalDue date:
Assignee:-% Done:

90%

Category:driver - device drivers
Target version:-
Difficulty:Bite-size Tags:

Description

Certain targets, such as those constructed by some Dell controllers, return NUL-padded VID and PID strings in their INQUIRY responses. SPC 6.6 and 4.3.1 and explicitly require that these fields be padded with spaces (ASCII 0x20). When sd encounters such a noncompliant device, sd-config-list processing does not work correctly and these devices generally will not be matched if the configuration template contains more characters than the device provides prior to the first NUL byte. The comment near 4458 in sd.c assumes a compliant device, and by modifying this code somewhat to eliminate this assumption, it's fairly easy to fix this problem without breaking compliant devices.


Related issues

Related to illumos gate - Feature #2665: sd.conf should be able to override physical-block-size Resolved 2012-05-01

History

#1 Updated by Keith Wesolowski about 6 years ago

A better fix (why can't I edit? blecch):

diff --git a/usr/src/uts/common/io/scsi/targets/sd.c b/usr/src/uts/common/io/scs
index 7533e4c..29291f4 100644
--- a/usr/src/uts/common/io/scsi/targets/sd.c
+++ b/usr/src/uts/common/io/scsi/targets/sd.c
@@ -4446,18 +4446,77 @@ sd_sdconf_id_match(struct sd_lun *un, char *id, int idle
 {
        struct scsi_inquiry     *sd_inq;
        int                     rval = SD_SUCCESS;
+       char                    *p;
+       int                     chk_vidlen, chk_pidlen;
+       int                     has_tail = 0;
+       static const int        VSZ = sizeof (sd_inq->inq_vid);
+       static const int        PSZ = sizeof (sd_inq->inq_pid);

        ASSERT(un != NULL);
        sd_inq = un->un_sd->sd_inq;
        ASSERT(id != NULL);

        /*
-        * We use the inq_vid as a pointer to a buffer containing the
-        * vid and pid and use the entire vid/pid length of the table
-        * entry for the comparison. This works because the inq_pid
-        * data member follows inq_vid in the scsi_inquiry structure.
+        * We would like to use the inq_vid as a pointer to a buffer
+        * containing the vid and pid and use the entire vid/pid length of
+        * the table entry for the comparison.  However, this does not work
+        * because, while the inq_pid data member follows inq_vid in the
+        * scsi_inquiry structure, we do not control the contents of this
+        * buffer, and some broken devices violate SPC 4.3.1 and return
+        * fields with null bytes in them.
         */
-       if (strncasecmp(sd_inq->inq_vid, id, idlen) != 0) {
+       chk_vidlen = MIN(VSZ, idlen);
+       p = id + chk_vidlen - 1;
+       while (*p == ' ' && chk_vidlen > 0) {
+               --p;
+               --chk_vidlen;
+       }
+
+       /*
+        * If it's all spaces, check the whole thing.
+        */
+       if (chk_vidlen == 0)
+               chk_vidlen = MIN(VSZ, idlen);
+
+       if (idlen > VSZ) {
+               chk_pidlen = idlen - VSZ;
+               p = id + idlen - 1;
+               while (*p == ' ' && chk_pidlen > 0) {
+                       --p;
+                       --chk_pidlen;
+               }
+               if (chk_pidlen == 0)
+                       chk_pidlen = MIN(PSZ, idlen - VSZ);
+       }
+
+       /*
+        * There's one more thing we need to do here.  If the user specified
+        * an ID with trailing spaces, we need to make sure the inquiry
+        * vid/pid has only spaces or NULs after the check length; otherwise, it
+        * can't match.
+        */
+       if (idlen > chk_vidlen && chk_vidlen < VSZ) {
+               for (p = sd_inq->inq_vid + chk_vidlen;
+                   p < sd_inq->inq_vid + VSZ; ++p) {
+                       if (*p != ' ' && *p != '\0') {
+                               ++has_tail;
+                               break;
+                       }
+               }
+       }
+       if (idlen > chk_pidlen + VSZ && chk_pidlen < PSZ) {
+               for (p = sd_inq->inq_pid + chk_pidlen;
+                   p < sd_inq->inq_pid + PSZ; ++p) {
+                       if (*p != ' ' && *p != '\0') {
+                               ++has_tail;
+                               break;
+                       }
+               }
+       }
+
+       if (has_tail || strncasecmp(sd_inq->inq_vid, id, chk_vidlen) != 0 ||
+           (idlen > VSZ &&
+           strncasecmp(sd_inq->inq_pid, id + VSZ, chk_pidlen) != 0)) {
                /*
                 * The user id string is compared to the inquiry vid/pid
                 * using a case insensitive comparison and ignoring

Also available in: Atom