| Message ID | 20230118012906.2376-1-liubo03@inspur.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | EDAC/mc_sysfs: use strscpy() to instead of strncpy() | expand |
> 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 --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 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(-)