diff mbox series

EDAC/mc_sysfs: use strscpy() to instead of strncpy()

Message ID 20230118012906.2376-1-liubo03@inspur.com
State New
Headers
Series EDAC/mc_sysfs: use strscpy() to instead of strncpy() |

Commit Message

Bo Liu Jan. 18, 2023, 1:29 a.m. UTC 
  The implementation of strscpy() is more robust and safer.
That's now the recommended way to copy NUL terminated strings.

Signed-off-by: Bo Liu <liubo03@inspur.com>
---
 drivers/edac/edac_mc_sysfs.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Qiuxu Zhuo Jan. 18, 2023, 4:55 a.m. UTC | #1 
> From: Bo Liu <liubo03@inspur.com>
> Sent: Wednesday, January 18, 2023 9:29 AM
> To: bp@alien8.de; Luck, Tony <tony.luck@intel.com>;
> james.morse@arm.com; mchehab@kernel.org; rric@kernel.org
> Cc: linux-edac@vger.kernel.org; linux-kernel@vger.kernel.org; Bo Liu
> <liubo03@inspur.com>
> Subject: [PATCH] EDAC/mc_sysfs: use strscpy() to instead of strncpy()
> 
> The implementation of strscpy() is more robust and safer.
> That's now the recommended way to copy NUL terminated strings.
> 
> Signed-off-by: Bo Liu <liubo03@inspur.com>
> ---
>  drivers/edac/edac_mc_sysfs.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
> index 15f63452a9be..b709cbe8dcf9 100644
> --- a/drivers/edac/edac_mc_sysfs.c
> +++ b/drivers/edac/edac_mc_sysfs.c
> @@ -229,8 +229,7 @@ static ssize_t channel_dimm_label_store(struct
> device *dev,
>  	if (copy_count == 0 || copy_count >= sizeof(rank->dimm->label))
>  		return -EINVAL;
> 
> -	strncpy(rank->dimm->label, data, copy_count);
> -	rank->dimm->label[copy_count] = '\0';
> +	strscpy(rank->dimm->label, data, copy_count + 1);
> 
>  	return count;
>  }
> @@ -535,8 +534,7 @@ static ssize_t dimmdev_label_store(struct device
> *dev,
>  	if (copy_count == 0 || copy_count >= sizeof(dimm->label))
>  		return -EINVAL;
> 
> -	strncpy(dimm->label, data, copy_count);
> -	dimm->label[copy_count] = '\0';
> +	strscpy(dimm->label, data, copy_count + 1);
> 
>  	return count;

The 'copy_count' variable is conditionally decreased by 1 in the original code, 
but 'strscpy(dimm->label, data, copy_count + 1)' unconditionally references the 'data' in the length of 'copy_count + 1'.

Logically, they're mismatched. 

If data[count - 1] != '\0' && data[count - 1] != '\n', it may result in an overflow on referencing the 'data'.

-Qiuxu
diff mbox series

Patch

diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 15f63452a9be..b709cbe8dcf9 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -229,8 +229,7 @@  static ssize_t channel_dimm_label_store(struct device *dev,
 	if (copy_count == 0 || copy_count >= sizeof(rank->dimm->label))
 		return -EINVAL;
 
-	strncpy(rank->dimm->label, data, copy_count);
-	rank->dimm->label[copy_count] = '\0';
+	strscpy(rank->dimm->label, data, copy_count + 1);
 
 	return count;
 }
@@ -535,8 +534,7 @@  static ssize_t dimmdev_label_store(struct device *dev,
 	if (copy_count == 0 || copy_count >= sizeof(dimm->label))
 		return -EINVAL;
 
-	strncpy(dimm->label, data, copy_count);
-	dimm->label[copy_count] = '\0';
+	strscpy(dimm->label, data, copy_count + 1);
 
 	return count;
 }