| Message ID | 20250212115557.111263-2-thorsten.blum@linux.dev (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: hpsa: Use min() to simplify code | expand |
On 2/12/25 3:55 AM, Thorsten Blum wrote: > Use min() to simplify the host_store_hp_ssd_smart_path_status() and > host_store_raid_offload_debug() functions. > > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> > --- > drivers/scsi/hpsa.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c > index 84d8de07b7ae..1d19eb2ca1d3 100644 > --- a/drivers/scsi/hpsa.c > +++ b/drivers/scsi/hpsa.c > @@ -460,7 +460,7 @@ static ssize_t host_store_hp_ssd_smart_path_status(struct device *dev, > > if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO)) > return -EACCES; > - len = count > sizeof(tmpbuf) - 1 ? sizeof(tmpbuf) - 1 : count; > + len = min(count, sizeof(tmpbuf) - 1); > strncpy(tmpbuf, buf, len); > tmpbuf[len] = '\0'; > if (sscanf(tmpbuf, "%d", &status) != 1) > @@ -484,7 +484,7 @@ static ssize_t host_store_raid_offload_debug(struct device *dev, > > if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO)) > return -EACCES; > - len = count > sizeof(tmpbuf) - 1 ? sizeof(tmpbuf) - 1 : count; > + len = min(count, sizeof(tmpbuf) - 1); > strncpy(tmpbuf, buf, len); > tmpbuf[len] = '\0'; > if (sscanf(tmpbuf, "%d", &debug_level) != 1) From Documentation/process/deprecated.rst: <quote> strncpy() on NUL-terminated strings ----------------------------------- Use of strncpy() does not guarantee that the destination buffer will be NUL terminated. This can lead to various linear read overflows and other misbehavior due to the missing termination. It also NUL-pads the destination buffer if the source contents are shorter than the destination buffer size, which may be a needless performance penalty for callers using only NUL-terminated strings. When the destination is required to be NUL-terminated, the replacement is strscpy(), though care must be given to any cases where the return value of strncpy() was used, since strscpy() does not return a pointer to the destination, but rather a count of non-NUL bytes copied (or negative errno when it truncates). Any cases still needing NUL-padding should instead use strscpy_pad(). If a caller is using non-NUL-terminated strings, strtomem() should be used, and the destinations should be marked with the `__nonstring <https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html>`_ attribute to avoid future compiler warnings. For cases still needing NUL-padding, strtomem_pad() can be used. </quote> Instead of only changing the calculation of 'len', please change the strncpy() calls into strscpy() calls. Thanks, Bart.
On 12. Feb 2025, at 19:48, Bart Van Assche wrote: > On 2/12/25 3:55 AM, Thorsten Blum wrote: >> Use min() to simplify the host_store_hp_ssd_smart_path_status() and >> host_store_raid_offload_debug() functions. >> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> >> --- > > From Documentation/process/deprecated.rst: > > <quote> > strncpy() on NUL-terminated strings > ----------------------------------- > Use of strncpy() does not guarantee that the destination buffer will > be NUL terminated. This can lead to various linear read overflows and > other misbehavior due to the missing termination. It also NUL-pads > the destination buffer if the source contents are shorter than the > destination buffer size, which may be a needless performance penalty > for callers using only NUL-terminated strings. > > When the destination is required to be NUL-terminated, the replacement is > strscpy(), though care must be given to any cases where the return value > of strncpy() was used, since strscpy() does not return a pointer to the > destination, but rather a count of non-NUL bytes copied (or negative > errno when it truncates). Any cases still needing NUL-padding should > instead use strscpy_pad(). > > If a caller is using non-NUL-terminated strings, strtomem() should be > used, and the destinations should be marked with the `__nonstring > <https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html>`_ > attribute to avoid future compiler warnings. For cases still needing > NUL-padding, strtomem_pad() can be used. > </quote> > > Instead of only changing the calculation of 'len', please change the > strncpy() calls into strscpy() calls. Thank you for the suggestion. I just sent a new patch [1] essentially replacing this one. Best, Thorsten [1] https://lore.kernel.org/r/20250212222214.86110-2-thorsten.blum@linux.dev/
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 84d8de07b7ae..1d19eb2ca1d3 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -460,7 +460,7 @@ static ssize_t host_store_hp_ssd_smart_path_status(struct device *dev, if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO)) return -EACCES; - len = count > sizeof(tmpbuf) - 1 ? sizeof(tmpbuf) - 1 : count; + len = min(count, sizeof(tmpbuf) - 1); strncpy(tmpbuf, buf, len); tmpbuf[len] = '\0'; if (sscanf(tmpbuf, "%d", &status) != 1) @@ -484,7 +484,7 @@ static ssize_t host_store_raid_offload_debug(struct device *dev, if (!capable(CAP_SYS_ADMIN) || !capable(CAP_SYS_RAWIO)) return -EACCES; - len = count > sizeof(tmpbuf) - 1 ? sizeof(tmpbuf) - 1 : count; + len = min(count, sizeof(tmpbuf) - 1); strncpy(tmpbuf, buf, len); tmpbuf[len] = '\0'; if (sscanf(tmpbuf, "%d", &debug_level) != 1)
Use min() to simplify the host_store_hp_ssd_smart_path_status() and host_store_raid_offload_debug() functions. Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> --- drivers/scsi/hpsa.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)