scsi: ufs: ufshcd: Remove VLA usage
PATCH 1/1 scsi: ufs: ufshcd: Remove VLA usage
On the quest to remove all VLAs from the kernel[1] this moves buffers off the stack. In the second instance, this collapses two separately allocated buffers into a single buffer, since they are used consecutively, which saves 256 bytes (QUERY_DESC_MAX_SIZE + 1) of stack space.
[1] https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com
--- drivers/scsi/ufs/ufshcd.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-)
drivers/scsi/ufs/ufshcd.c +26 -8
@@ -5958,14 +5958,18 @@ static void ufshcd_init_icc_levels(struct ufs_hba *hba) {int ret;int buff_len = hba->desc_size.pwr_desc; -u8 desc_buf[hba->desc_size.pwr_desc]; +u8 *desc_buf; + + desc_buf =kmalloc (buff_len,GFP_KERNEL ); +if (!desc_buf) +return ; ret =ufshcd_read_power_desc (hba, desc_buf, buff_len);if (ret) {dev_err (hba->dev,"%s: Failed reading power descriptor.len = %d ret = %d" , __func__, buff_len, ret); -return ; + goto out; } hba->init_prefetch_data.icc_level =
@@ -5983,6 +5987,8 @@ static void ufshcd_init_icc_levels(struct ufs_hba *hba)"%s: Failed configuring bActiveICCLevel = %d ret = %d" , __func__, hba->init_prefetch_data.icc_level , ret); +out: +kfree (desc_buf); }/**
@@ -6052,9 +6058,17 @@ static int ufs_get_device_desc(struct ufs_hba *hba, struct ufs_dev_desc *dev_desc) {int err; +size_t buff_len;u8 model_index; -u8 str_desc_buf[QUERY_DESC_MAX_SIZE +1 ] = {0 }; -u8 desc_buf[hba->desc_size.dev_desc]; +u8 *desc_buf; + + buff_len =max_t (size_t, hba->desc_size.dev_desc, +QUERY_DESC_MAX_SIZE +1 ); + desc_buf =kmalloc (buff_len,GFP_KERNEL ); +if (!desc_buf) { + err = -ENOMEM ; + goto out; + } err =ufshcd_read_device_desc (hba, desc_buf, hba->desc_size.dev_desc);if (err) {
@@ -6072,7 +6086,10 @@ static int ufs_get_device_desc(struct ufs_hba *hba, model_index = desc_buf[DEVICE_DESC_PARAM_PRDCT_NAME ]; - err =ufshcd_read_string_desc (hba, model_index, str_desc_buf, +/* 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) { dev_err(hba->dev, "%s: Failed reading Product Name. err = %d\n",
@@ -6080,15 +6097,16 @@ static int ufs_get_device_desc(struct ufs_hba *hba, goto out; } - str_desc_buf[QUERY_DESC_MAX_SIZE ] ='\0' ; -strlcpy (dev_desc->model, (str_desc_buf +QUERY_DESC_HDR_SIZE ), -min_t (u8, str_desc_buf[QUERY_DESC_LENGTH_OFFSET ], + 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' ; out: +kfree (desc_buf);return err; }
Signature · 6 lines
2.17.0 -- Kees Cook Pixel Security
Quoted 106 lines · Kees Cook
On the quest to remove all VLAs from the kernel[1] this moves buffers off the stack. In the second instance, this collapses two separately allocated buffers into a single buffer, since they are used consecutively, which saves 256 bytes (
QUERY_DESC_MAX_SIZE+ 1) of stack space.[1]
https://lkml.kernel.org/r/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.comSigned-off-by Kees Cook <[email protected]>--- drivers/scsi/ufs/ufshcd.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-)drivers/scsi/ufs/ufshcd.c +6
-2@@ -5958,14 +5958,18 @@ static void ufshcd_init_icc_levels(struct ufs_hba *hba) {int ret;int buff_len = hba->desc_size.pwr_desc; -u8 desc_buf[hba->desc_size.pwr_desc]; +u8 *desc_buf; + + desc_buf =kmalloc (buff_len,GFP_KERNEL ); +if (!desc_buf) +return ; ret =ufshcd_read_power_desc (hba, desc_buf, buff_len);if (ret) {dev_err (hba->dev,"%s: Failed reading power descriptor.len = %d ret = %d" , __func__, buff_len, ret); -return ; + goto out; }drivers/scsi/ufs/ufshcd.c +2
-0hba->init_prefetch_data.icc_level =@@ -5983,6 +5987,8 @@ static void ufshcd_init_icc_levels(struct ufs_hba *hba)"%s: Failed configuring bActiveICCLevel = %d ret = %d" , __func__, hba->init_prefetch_data.icc_level , ret); +out: +kfree (desc_buf); }/**
drivers/scsi/ufs/ufshcd.c +10
-2@@ -6052,9 +6058,17 @@ static int ufs_get_device_desc(struct ufs_hba *hba, struct ufs_dev_desc *dev_desc) {int err; +size_t buff_len;u8 model_index; -u8 str_desc_buf[QUERY_DESC_MAX_SIZE +1 ] = {0 }; -u8 desc_buf[hba->desc_size.dev_desc]; +u8 *desc_buf; + + buff_len =max_t (size_t, hba->desc_size.dev_desc, +QUERY_DESC_MAX_SIZE +1 ); + desc_buf =kmalloc (buff_len,GFP_KERNEL ); +if (!desc_buf) { + err = -ENOMEM ; + goto out; + } err = ufshcd_read_device_desc(hba, desc_buf,
hba->desc_size.dev_desc);drivers/scsi/ufs/ufshcd.c +4
-1if (err) {@@ -6072,7 +6086,10 @@ static int ufs_get_device_desc(struct ufs_hba *hba, model_index = desc_buf[DEVICE_DESC_PARAM_PRDCT_NAME ]; - err =ufshcd_read_string_desc (hba, model_index, str_desc_buf, +/* 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) {drivers/scsi/ufs/ufshcd.c +4
-4dev_err(hba->dev, "%s: Failed reading Product Name. err = %d\n",@@ -6080,15 +6097,16 @@ static int ufs_get_device_desc(struct ufs_hba *hba, goto out; } - str_desc_buf[QUERY_DESC_MAX_SIZE ] ='\0' ; -strlcpy (dev_desc->model, (str_desc_buf +QUERY_DESC_HDR_SIZE ), -min_t (u8, str_desc_buf[QUERY_DESC_LENGTH_OFFSET ], + 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' ; out: +kfree (desc_buf);return err; } --
Quoted 1 line
2.17.0
Looks good to me.
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Martin K. Petersen
AppliedKees,
Quoted 5 lines
On the quest to remove all VLAs from the kernel[1] this moves buffers off the stack. In the second instance, this collapses two separately allocated buffers into a single buffer, since they are used consecutively, which saves 256 bytes (
QUERY_DESC_MAX_SIZE+ 1) of stack space.
Applied to 4.18/scsi-queue. Thank you!
Signature · 1 line
Martin K. Petersen Oracle Linux Engineering