scsi: target: consistently null-terminate t10_wwn strings

In preparation for supporting user provided vendor strings, add an extra
byte to the vendor, model and revision arrays in struct t10_wwn. This
ensures that the full INQUIRY data can be carried in the arrays along with
a null-terminator.

Change a number of array readers and writers so that they account for
explicit null-termination:

- The pscsi_set_inquiry_info() and emulate_model_alias_store() codepaths
  don't currently explicitly null-terminate; fix this.

- Existing t10_wwn field dumps use for-loops which step over
  null-terminators for right-padding.
  + Use printf with width specifiers instead.

This commit was backported by hand.

Orabug: 29344862

Signed-off-by: Alan Adamson <alan.adamson@oracle.com>
Reviewed-by: Bijan Mottahedeh <bijan.mottahedeh@oracle.com>
Signed-off-by: Brian Maly <brian.maly@oracle.com>

diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 193d95d35dbbb5969ddca3f5eadee1f3912cae9b..13dedb504d52cf923a73da308f9c637ea8d30288 100644 (file)
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -1456,36 +1456,17 @@ void core_dev_free_initiator_node_lun_acl(
 static void scsi_dump_inquiry(struct se_device *dev)
 {
        struct t10_wwn *wwn = &dev->t10_wwn;
-       char buf[17];
-       int i, device_type;
+       int device_type = dev->transport->get_device_type(dev);
+
        /*
         * Print Linux/SCSI style INQUIRY formatting to the kernel ring buffer
         */
-       for (i = 0; i < 8; i++)
-               if (wwn->vendor[i] >= 0x20)
-                       buf[i] = wwn->vendor[i];
-               else
-                       buf[i] = ' ';
-       buf[i] = '\0';
-       pr_debug("  Vendor: %s\n", buf);
-
-       for (i = 0; i < 16; i++)
-               if (wwn->model[i] >= 0x20)
-                       buf[i] = wwn->model[i];
-               else
-                       buf[i] = ' ';
-       buf[i] = '\0';
-       pr_debug("  Model: %s\n", buf);
-
-       for (i = 0; i < 4; i++)
-               if (wwn->revision[i] >= 0x20)
-                       buf[i] = wwn->revision[i];
-               else
-                       buf[i] = ' ';
-       buf[i] = '\0';
-       pr_debug("  Revision: %s\n", buf);
-
-       device_type = dev->transport->get_device_type(dev);
+       pr_debug("  Vendor: %-" __stringify(INQUIRY_VENDOR_LEN) "s\n",
+               wwn->vendor);
+       pr_debug("  Model: %-" __stringify(INQUIRY_MODEL_LEN) "s\n",
+               wwn->model);
+       pr_debug("  Revision: %-" __stringify(INQUIRY_REVISION_LEN) "s\n",
+               wwn->revision);
        pr_debug("  Type:   %s ", scsi_device_type(device_type));
 }
 
@@ -1626,18 +1607,12 @@ int target_configure_device(struct se_device *dev)
         * passthrough because this is being provided by the backend LLD.
         */
        if (!(dev->transport->transport_flags & TRANSPORT_FLAG_PASSTHROUGH)) {
-               strncpy(&dev->t10_wwn.vendor[0], g_inquiry_vendor, 8);
-               /*
-                * If module_parm inquiry_product was set, use that.
-                */
-               if (g_inquiry_product[0] != '\0')
-                       strncpy(&dev->t10_wwn.model[0],
-                               g_inquiry_product, 16);
-               else
-                       strncpy(&dev->t10_wwn.model[0],
-                               dev->transport->inquiry_prod, 16);
-               strncpy(&dev->t10_wwn.revision[0],
-                       dev->transport->inquiry_rev, 4);
+               strlcpy(dev->t10_wwn.vendor, "LIO-ORG",
+                       sizeof(dev->t10_wwn.vendor));
+               strlcpy(dev->t10_wwn.model, dev->transport->inquiry_prod,
+                       sizeof(dev->t10_wwn.model));
+               strlcpy(dev->t10_wwn.revision, dev->transport->inquiry_rev,
+                       sizeof(dev->t10_wwn.revision));
        }
 
        scsi_dump_inquiry(dev);

diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index d2493ee5bbb15a9c3962ac53cdcf79a81ae5fec7..a2dbd7c28e69efe447548454b854ae4a0d7e7241 100644 (file)
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -179,20 +179,20 @@ out_free:
 static void
 pscsi_set_inquiry_info(struct scsi_device *sdev, struct t10_wwn *wwn)
 {
-       unsigned char *buf;
-
        if (sdev->inquiry_len < INQUIRY_LEN)
                return;
-
-       buf = sdev->inquiry;
-       if (!buf)
-               return;
        /*
-        * Use sdev->inquiry from drivers/scsi/scsi_scan.c:scsi_alloc_sdev()
+        * Use sdev->inquiry data from drivers/scsi/scsi_scan.c:scsi_add_lun()
         */
-       memcpy(&wwn->vendor[0], &buf[8], sizeof(wwn->vendor));
-       memcpy(&wwn->model[0], &buf[16], sizeof(wwn->model));
-       memcpy(&wwn->revision[0], &buf[32], sizeof(wwn->revision));
+       BUILD_BUG_ON(sizeof(wwn->vendor) != INQUIRY_VENDOR_LEN + 1);
+       snprintf(wwn->vendor, sizeof(wwn->vendor),
+               "%." __stringify(INQUIRY_VENDOR_LEN) "s", sdev->vendor);
+       BUILD_BUG_ON(sizeof(wwn->model) != INQUIRY_MODEL_LEN + 1);
+       snprintf(wwn->model, sizeof(wwn->model),
+               "%." __stringify(INQUIRY_MODEL_LEN) "s", sdev->model);
+       BUILD_BUG_ON(sizeof(wwn->revision) != INQUIRY_REVISION_LEN + 1);
+       snprintf(wwn->revision, sizeof(wwn->revision),
+               "%." __stringify(INQUIRY_REVISION_LEN) "s", sdev->rev);
 }
 
 static int
@@ -804,7 +804,6 @@ static ssize_t pscsi_show_configfs_dev_params(struct se_device *dev, char *b)
        struct scsi_device *sd = pdv->pdv_sd;
        unsigned char host_id[16];
        ssize_t bl;
-       int i;
 
        if (phv->phv_mode == PHV_VIRTUAL_HOST_ID)
                snprintf(host_id, 16, "%d", pdv->pdv_host_id);
@@ -817,29 +816,12 @@ static ssize_t pscsi_show_configfs_dev_params(struct se_device *dev, char *b)
                host_id);
 
        if (sd) {
-               bl += sprintf(b + bl, "        ");
-               bl += sprintf(b + bl, "Vendor: ");
-               for (i = 0; i < 8; i++) {
-                       if (ISPRINT(sd->vendor[i]))   /* printable character? */
-                               bl += sprintf(b + bl, "%c", sd->vendor[i]);
-                       else
-                               bl += sprintf(b + bl, " ");
-               }
-               bl += sprintf(b + bl, " Model: ");
-               for (i = 0; i < 16; i++) {
-                       if (ISPRINT(sd->model[i]))   /* printable character ? */
-                               bl += sprintf(b + bl, "%c", sd->model[i]);
-                       else
-                               bl += sprintf(b + bl, " ");
-               }
-               bl += sprintf(b + bl, " Rev: ");
-               for (i = 0; i < 4; i++) {
-                       if (ISPRINT(sd->rev[i]))   /* printable character ? */
-                               bl += sprintf(b + bl, "%c", sd->rev[i]);
-                       else
-                               bl += sprintf(b + bl, " ");
-               }
-               bl += sprintf(b + bl, "\n");
+               bl += sprintf(b + bl, "        Vendor: %."
+                       __stringify(INQUIRY_VENDOR_LEN) "s", sd->vendor);
+               bl += sprintf(b + bl, " Model: %."
+                       __stringify(INQUIRY_MODEL_LEN) "s", sd->model);
+               bl += sprintf(b + bl, " Rev: %."
+                       __stringify(INQUIRY_REVISION_LEN) "s\n", sd->rev);
        }
        return bl;
 }

diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index bfe2ee362531f46f6bbb342c814cce9446c22c82..313145294dfd73a8d9c6cd3db90dea2ef66135be 100644 (file)
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -120,12 +120,13 @@ spc_emulate_inquiry_std(struct se_cmd *cmd, unsigned char *buf)
         * unused bytes at the end of the field (i.e., highest offset) and the
         * unused bytes shall be filled with ASCII space characters (20h).
         */
-       memset(&buf[8], 0x20, 8 + 16 + 4);
+       memset(&buf[8], 0x20,
+               INQUIRY_VENDOR_LEN + INQUIRY_MODEL_LEN + INQUIRY_REVISION_LEN);
        memcpy(&buf[8], "LIO-ORG", sizeof("LIO-ORG") - 1);
        memcpy(&buf[16], dev->t10_wwn.model,
-              strnlen(dev->t10_wwn.model, 16));
+               strnlen(dev->t10_wwn.model, INQUIRY_MODEL_LEN));
        memcpy(&buf[32], dev->t10_wwn.revision,
-              strnlen(dev->t10_wwn.revision, 4));
+               strnlen(dev->t10_wwn.revision, INQUIRY_REVISION_LEN));
        buf[4] = 31; /* Set additional length to 31 */
 
        return 0;

diff --git a/drivers/target/target_core_stat.c b/drivers/target/target_core_stat.c
index 03538994d2f7e1ee817d5b594fd33174079abaf3..b344686ea777b17fadec6c7476644007a4eb9855 100644 (file)
--- a/drivers/target/target_core_stat.c
+++ b/drivers/target/target_core_stat.c
@@ -312,15 +312,9 @@ static ssize_t target_stat_scsi_lu_show_attr_vend(
 {
        struct se_device *dev =
                container_of(sgrps, struct se_device, dev_stat_grps);
-       int i;
-       char str[sizeof(dev->t10_wwn.vendor)+1];
 
-       /* scsiLuVendorId */
-       for (i = 0; i < sizeof(dev->t10_wwn.vendor); i++)
-               str[i] = ISPRINT(dev->t10_wwn.vendor[i]) ?
-                       dev->t10_wwn.vendor[i] : ' ';
-       str[i] = '\0';
-       return snprintf(page, PAGE_SIZE, "%s\n", str);
+       return snprintf(page, PAGE_SIZE, "%-" __stringify(INQUIRY_VENDOR_LEN)
+                       "s\n", dev->t10_wwn.vendor);
 }
 DEV_STAT_SCSI_LU_ATTR_RO(vend);
 
@@ -329,15 +323,9 @@ static ssize_t target_stat_scsi_lu_show_attr_prod(
 {
        struct se_device *dev =
                container_of(sgrps, struct se_device, dev_stat_grps);
-       int i;
-       char str[sizeof(dev->t10_wwn.model)+1];
 
-       /* scsiLuProductId */
-       for (i = 0; i < sizeof(dev->t10_wwn.vendor); i++)
-               str[i] = ISPRINT(dev->t10_wwn.model[i]) ?
-                       dev->t10_wwn.model[i] : ' ';
-       str[i] = '\0';
-       return snprintf(page, PAGE_SIZE, "%s\n", str);
+       return snprintf(page, PAGE_SIZE, "%-" __stringify(INQUIRY_MODEL_LEN)
+                       "s\n", dev->t10_wwn.model);
 }
 DEV_STAT_SCSI_LU_ATTR_RO(prod);
 
@@ -346,15 +334,9 @@ static ssize_t target_stat_scsi_lu_show_attr_rev(
 {
        struct se_device *dev =
                container_of(sgrps, struct se_device, dev_stat_grps);
-       int i;
-       char str[sizeof(dev->t10_wwn.revision)+1];
-
-       /* scsiLuRevisionId */
-       for (i = 0; i < sizeof(dev->t10_wwn.revision); i++)
-               str[i] = ISPRINT(dev->t10_wwn.revision[i]) ?
-                       dev->t10_wwn.revision[i] : ' ';
-       str[i] = '\0';
-       return snprintf(page, PAGE_SIZE, "%s\n", str);
+
+       return snprintf(page, PAGE_SIZE, "%-" __stringify(INQUIRY_REVISION_LEN)
+                       "s\n", dev->t10_wwn.revision);
 }
 DEV_STAT_SCSI_LU_ATTR_RO(rev);
 

diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index 60345d151ad02304387e9786fce2c9e44e9fe8cb..4473174478080f9d30f60fa9dc93dec253c41b22 100644 (file)
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -59,6 +59,10 @@
 /* Used by transport_get_inquiry_vpd_device_ident() */
 #define INQUIRY_VPD_DEVICE_IDENTIFIER_LEN      254
 
+#define INQUIRY_VENDOR_LEN                     8
+#define INQUIRY_MODEL_LEN                      16
+#define INQUIRY_REVISION_LEN                   4
+
 /* Attempts before moving from SHORT to LONG */
 #define PYX_TRANSPORT_WINDOW_CLOSED_THRESHOLD  3
 #define PYX_TRANSPORT_WINDOW_CLOSED_WAIT_SHORT 3  /* In milliseconds */
@@ -351,9 +355,9 @@ struct t10_vpd {
 };
 
 struct t10_wwn {
-       char vendor[8];
-       char model[16];
-       char revision[4];
+       char vendor[INQUIRY_VENDOR_LEN + 1];
+       char model[INQUIRY_MODEL_LEN + 1];
+       char revision[INQUIRY_REVISION_LEN + 1];
        char unit_serial[INQUIRY_VPD_SERIAL_LEN];
        spinlock_t t10_vpd_lock;
        struct se_device *t10_dev;