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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <IA1PR11MB61718652054C38D8881F3F6289C79@IA1PR11MB6171.namprd11.prod.outlook.com>
Date:   Wed, 18 Jan 2023 04:55:15 +0000
From:   "Zhuo, Qiuxu" <qiuxu.zhuo@...el.com>
To:     Bo Liu <liubo03@...pur.com>, "bp@...en8.de" <bp@...en8.de>,
        "Luck, Tony" <tony.luck@...el.com>,
        "james.morse@....com" <james.morse@....com>,
        "mchehab@...nel.org" <mchehab@...nel.org>,
        "rric@...nel.org" <rric@...nel.org>
CC:     "linux-edac@...r.kernel.org" <linux-edac@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] EDAC/mc_sysfs: use strscpy() to instead of strncpy()

> From: Bo Liu <liubo03@...pur.com>
> Sent: Wednesday, January 18, 2023 9:29 AM
> To: bp@...en8.de; Luck, Tony <tony.luck@...el.com>;
> james.morse@....com; mchehab@...nel.org; rric@...nel.org
> Cc: linux-edac@...r.kernel.org; linux-kernel@...r.kernel.org; Bo Liu
> <liubo03@...pur.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@...pur.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