From: David Disseldorp Date: Thu, 7 Mar 2019 22:36:13 +0000 (-0800) Subject: scsi: target: consistently null-terminate t10_wwn strings X-Git-Tag: v4.1.12-124.31.3~205 X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=d85e31b03db1b914e30d56e991e8a276998f6e67;p=users%2Fjedix%2Flinux-maple.git 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 Reviewed-by: Bijan Mottahedeh Signed-off-by: Brian Maly --- diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 193d95d35dbb..13dedb504d52 100644 --- 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 d2493ee5bbb1..a2dbd7c28e69 100644 --- 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 bfe2ee362531..313145294dfd 100644 --- 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 03538994d2f7..b344686ea777 100644 --- 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 60345d151ad0..447317447808 100644 --- 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;