RE: [PATCH] EDAC/mc_sysfs: use strscpy() to instead of strncpy()

[Zhuo, Qiuxu]

> From: Bo Liu <liubo03@xxxxxxxxxx>
> Sent: Wednesday, January 18, 2023 9:29 AM
> To: bp@xxxxxxxxx; Luck, Tony <tony.luck@xxxxxxxxx>;
> james.morse@xxxxxxx; mchehab@xxxxxxxxxx; rric@xxxxxxxxxx
> Cc: linux-edac@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Bo Liu
> <liubo03@xxxxxxxxxx>
> 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@xxxxxxxxxx>
> ---
>  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