From 17207cc752aff4f8d4d5b2243e2baf6edbc5ed87 Mon Sep 17 00:00:00 2001 From: Tomas Winkler Date: Wed, 4 Jan 2017 14:14:21 +0200 Subject: [PATCH 024/550] scsi: ufs: revamp string descriptor reading Define new a type: uc_string_id for easier string handling and less casting. Reduce number or string copies in price of a dynamic allocation. In addition drop usage of variable length array (VLA) as it's not considered to be safe. V9: Fix memory corruption. Change-Id: Ieda6a4b68e60b8a2d8a2d93a371ff5396dec989b Signed-off-by: Tomas Winkler --- drivers/scsi/ufs/ufs-sysfs.c | 20 ++--- drivers/scsi/ufs/ufs.h | 2 +- drivers/scsi/ufs/ufshcd.c | 163 +++++++++++++++++++++-------------- drivers/scsi/ufs/ufshcd.h | 9 +- 4 files changed, 114 insertions(+), 80 deletions(-) diff --git a/drivers/scsi/ufs/ufs-sysfs.c b/drivers/scsi/ufs/ufs-sysfs.c index 8d9332bb7d0c..0b221c5a244c 100644 --- a/drivers/scsi/ufs/ufs-sysfs.c +++ b/drivers/scsi/ufs/ufs-sysfs.c @@ -570,10 +570,11 @@ static ssize_t _name##_show(struct device *dev, \ struct ufs_hba *hba = dev_get_drvdata(dev); \ int ret; \ int desc_len = QUERY_DESC_MAX_SIZE; \ - u8 *desc_buf; \ + char *desc_buf; \ + \ desc_buf = kzalloc(QUERY_DESC_MAX_SIZE, GFP_ATOMIC); \ - if (!desc_buf) \ - return -ENOMEM; \ + if (!desc_buf) \ + return -ENOMEM; \ ret = ufshcd_query_descriptor_retry(hba, \ UPIU_QUERY_OPCODE_READ_DESC, QUERY_DESC_IDN_DEVICE, \ 0, 0, desc_buf, &desc_len); \ @@ -582,14 +583,13 @@ static ssize_t _name##_show(struct device *dev, \ goto out; \ } \ index = desc_buf[DEVICE_DESC_PARAM##_pname]; \ - memset(desc_buf, 0, QUERY_DESC_MAX_SIZE); \ - if (ufshcd_read_string_desc(hba, index, desc_buf, \ - QUERY_DESC_MAX_SIZE, true)) { \ - ret = -EINVAL; \ + kfree(desc_buf); \ + desc_buf = NULL; \ + ret = ufshcd_read_string_desc(hba, index, &desc_buf, \ + SD_ASCII_STD); \ + if (ret < 0) \ goto out; \ - } \ - ret = snprintf(buf, PAGE_SIZE, "%s\n", \ - desc_buf + QUERY_DESC_HDR_SIZE); \ + ret = snprintf(buf, PAGE_SIZE, "%s\n", desc_buf); \ out: \ kfree(desc_buf); \ return ret; \ diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h index 14e5bf7af0bb..c9774b59b6bf 100644 --- a/drivers/scsi/ufs/ufs.h +++ b/drivers/scsi/ufs/ufs.h @@ -617,7 +617,7 @@ struct ufs_dev_info { */ struct ufs_dev_desc { u16 wmanufacturerid; - char model[MAX_MODEL_LEN + 1]; + char *model; }; /** diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index c55f38ec391c..0ee9a4baaeb8 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -297,16 +297,6 @@ static void ufshcd_scsi_block_requests(struct ufs_hba *hba) scsi_block_requests(hba->host); } -/* replace non-printable or non-ASCII characters with spaces */ -static inline void ufshcd_remove_non_printable(char *val) -{ - if (!val) - return; - - if (*val < 0x20 || *val > 0x7e) - *val = ' '; -} - static void ufshcd_add_cmd_upiu_trace(struct ufs_hba *hba, unsigned int tag, const char *str) { @@ -3120,7 +3110,7 @@ int ufshcd_read_desc_param(struct ufs_hba *hba, enum desc_idn desc_id, int desc_index, u8 param_offset, - u8 *param_read_buf, + void *param_read_buf, u8 param_size) { int ret; @@ -3188,7 +3178,7 @@ int ufshcd_read_desc_param(struct ufs_hba *hba, static inline int ufshcd_read_desc(struct ufs_hba *hba, enum desc_idn desc_id, int desc_index, - u8 *buf, + void *buf, u32 size) { return ufshcd_read_desc_param(hba, desc_id, desc_index, 0, buf, size); @@ -3206,49 +3196,77 @@ static int ufshcd_read_device_desc(struct ufs_hba *hba, u8 *buf, u32 size) return ufshcd_read_desc(hba, QUERY_DESC_IDN_DEVICE, 0, buf, size); } +/** + * struct uc_string_id - unicode string + * + * @len: size of this descriptor inclusive + * @type: descriptor type + * @uc: unicode string character + */ +struct uc_string_id { + u8 len; + u8 type; + wchar_t uc[0]; +} __packed; + +/* replace non-printable or non-ASCII characters with spaces */ +static inline char blank_non_printable(char ch) +{ + return (ch >= 0x20 && ch <= 0x7e) ? ch : ' '; +} + /** * ufshcd_read_string_desc - read string descriptor * @hba: pointer to adapter instance * @desc_index: descriptor index - * @buf: pointer to buffer where descriptor would be read - * @size: size of buf + * @buf: pointer to buffer where descriptor would be read, + * the caller should free the memory. * @ascii: if true convert from unicode to ascii characters + * null terminated string. * - * Return 0 in case of success, non-zero otherwise + * Return: string size on success. + * -ENOMEM: on allocation failure + * -EINVAL: on a wrong parameter */ -int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index, - u8 *buf, u32 size, bool ascii) +int ufshcd_read_string_desc(struct ufs_hba *hba, u8 desc_index, + char **buf, bool ascii) { - int err = 0; + struct uc_string_id *uc_str; + char *str; + int ret; - err = ufshcd_read_desc(hba, - QUERY_DESC_IDN_STRING, desc_index, buf, size); + if (!buf) + return -EINVAL; - if (err) { - dev_err(hba->dev, "%s: reading String Desc failed after %d retries. err = %d\n", - __func__, QUERY_REQ_RETRIES, err); + uc_str = kzalloc(QUERY_DESC_MAX_SIZE, GFP_KERNEL); + if (!uc_str) + return -ENOMEM; + + ret = ufshcd_read_desc(hba, QUERY_DESC_IDN_STRING, + desc_index, uc_str, + QUERY_DESC_MAX_SIZE); + if (ret < 0) { + dev_err(hba->dev, "Reading String Desc failed after %d retries. err = %d\n", + QUERY_REQ_RETRIES, ret); + str = NULL; + goto out; + } + + if (uc_str->len <= QUERY_DESC_HDR_SIZE) { + dev_dbg(hba->dev, "String Desc is of zero length\n"); + str = NULL; + ret = 0; goto out; } if (ascii) { - int desc_len; - int ascii_len; + ssize_t ascii_len; int i; - char *buff_ascii; - - desc_len = buf[0]; /* remove header and divide by 2 to move from UTF16 to UTF8 */ - ascii_len = (desc_len - QUERY_DESC_HDR_SIZE) / 2 + 1; - if (size < ascii_len + QUERY_DESC_HDR_SIZE) { - dev_err(hba->dev, "%s: buffer allocated size is too small\n", - __func__); - err = -ENOMEM; - goto out; - } - - buff_ascii = kmalloc(ascii_len, GFP_KERNEL); - if (!buff_ascii) { - err = -ENOMEM; + ascii_len = (uc_str->len - QUERY_DESC_HDR_SIZE) / 2 + 1; + str = kzalloc(ascii_len, GFP_KERNEL); + if (!str) { + ret = -ENOMEM; goto out; } @@ -3256,22 +3274,29 @@ int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index, * the descriptor contains string in UTF16 format * we need to convert to utf-8 so it can be displayed */ - utf16s_to_utf8s((wchar_t *)&buf[QUERY_DESC_HDR_SIZE], - desc_len - QUERY_DESC_HDR_SIZE, - UTF16_BIG_ENDIAN, buff_ascii, ascii_len); + ret = utf16s_to_utf8s(uc_str->uc, + uc_str->len - QUERY_DESC_HDR_SIZE, + UTF16_BIG_ENDIAN, str, ascii_len); /* replace non-printable or non-ASCII characters with spaces */ - for (i = 0; i < ascii_len; i++) - ufshcd_remove_non_printable(&buff_ascii[i]); + for (i = 0; i < ret; i++) + str[i] = blank_non_printable(str[i]); - memset(buf + QUERY_DESC_HDR_SIZE, 0, - size - QUERY_DESC_HDR_SIZE); - memcpy(buf + QUERY_DESC_HDR_SIZE, buff_ascii, ascii_len); - buf[QUERY_DESC_LENGTH_OFFSET] = ascii_len + QUERY_DESC_HDR_SIZE; - kfree(buff_ascii); + str[ret++] = '\0'; + + } else { + str = kzalloc(uc_str->len, GFP_KERNEL); + if (!str) { + ret = -ENOMEM; + goto out; + } + memcpy(str, uc_str, uc_str->len); + ret = uc_str->len; } out: - return err; + *buf = str; + kfree(uc_str); + return ret; } /** @@ -6250,6 +6275,9 @@ static int ufs_get_device_desc(struct ufs_hba *hba, u8 model_index; u8 *desc_buf; + if (!dev_desc) + return -EINVAL; + buff_len = max_t(size_t, hba->desc_size.dev_desc, QUERY_DESC_MAX_SIZE + 1); desc_buf = kmalloc(buff_len, GFP_KERNEL); @@ -6273,31 +6301,31 @@ static int ufs_get_device_desc(struct ufs_hba *hba, desc_buf[DEVICE_DESC_PARAM_MANF_ID + 1]; model_index = desc_buf[DEVICE_DESC_PARAM_PRDCT_NAME]; - - /* Zero-pad entire buffer for string termination. */ - memset(desc_buf, 0, buff_len); - - err = ufshcd_read_string_desc(hba, model_index, desc_buf, - QUERY_DESC_MAX_SIZE, true/*ASCII*/); - if (err) { + err = ufshcd_read_string_desc(hba, model_index, + &dev_desc->model, SD_ASCII_STD); + if (err < 0) { dev_err(hba->dev, "%s: Failed reading Product Name. err = %d\n", __func__, err); goto out; } - desc_buf[QUERY_DESC_MAX_SIZE] = '\0'; - strlcpy(dev_desc->model, (desc_buf + QUERY_DESC_HDR_SIZE), - min_t(u8, desc_buf[QUERY_DESC_LENGTH_OFFSET], - MAX_MODEL_LEN)); - - /* Null terminate the model string */ - dev_desc->model[MAX_MODEL_LEN] = '\0'; + /* + * ufshcd_read_string_desc returns size of the string + * reset the error value + */ + err = 0; out: kfree(desc_buf); return err; } +static void ufs_put_device_desc(struct ufs_dev_desc *dev_desc) +{ + kfree(dev_desc->model); + dev_desc->model = NULL; +} + static void ufs_fixup_device_setup(struct ufs_hba *hba, struct ufs_dev_desc *dev_desc) { @@ -6306,8 +6334,9 @@ static void ufs_fixup_device_setup(struct ufs_hba *hba, for (f = ufs_fixups; f->quirk; f++) { if ((f->card.wmanufacturerid == dev_desc->wmanufacturerid || f->card.wmanufacturerid == UFS_ANY_VENDOR) && - (STR_PRFX_EQUAL(f->card.model, dev_desc->model) || - !strcmp(f->card.model, UFS_ANY_MODEL))) + ((dev_desc->model && + STR_PRFX_EQUAL(f->card.model, dev_desc->model)) || + !strcmp(f->card.model, UFS_ANY_MODEL))) hba->dev_quirks |= f->quirk; } } @@ -6590,6 +6619,8 @@ static int ufshcd_probe_hba(struct ufs_hba *hba) } ufs_fixup_device_setup(hba, &card); + ufs_put_device_desc(&card); + ufshcd_tune_unipro_params(hba); ret = ufshcd_set_vccq_rail_unused(hba, diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index 33fdd3f281ae..c04bdbb1ea57 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -875,14 +875,17 @@ int ufshcd_read_desc_param(struct ufs_hba *hba, enum desc_idn desc_id, int desc_index, u8 param_offset, - u8 *param_read_buf, + void *param_read_buf, u8 param_size); int ufshcd_query_attr(struct ufs_hba *hba, enum query_opcode opcode, enum attr_idn idn, u8 index, u8 selector, u32 *attr_val); int ufshcd_query_flag(struct ufs_hba *hba, enum query_opcode opcode, enum flag_idn idn, bool *flag_res); -int ufshcd_read_string_desc(struct ufs_hba *hba, int desc_index, - u8 *buf, u32 size, bool ascii); + +#define SD_ASCII_STD true +#define SD_RAW false +int ufshcd_read_string_desc(struct ufs_hba *hba, u8 desc_index, + char **buf, bool ascii); int ufshcd_hold(struct ufs_hba *hba, bool async); void ufshcd_release(struct ufs_hba *hba); -- 2.19.1