| Message ID | 20240223-strncpy-drivers-scsi-mpi3mr-mpi3mr_fw-c-v1-5-9cd3882f0700@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: replace deprecated strncpy | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Fri, Feb 23, 2024 at 10:23:10PM +0000, Justin Stitt wrote: > Depending on the state of @compatible, we are going to do different > things with our @to buffer. > > When @compatible is true we want a NUL-term'd and NUL-padded destination > buffer. Conversely, if @compatible is false we just want a space-padded > destination buffer (no NUL-term required). > > As per: > /** > * scsi_dev_info_list_add_keyed - add one dev_info list entry. > * @compatible: if true, null terminate short strings. Otherwise space pad. > ... > > Note that we can't easily use `strtomem_pad` here as the size of the @to > buffer is unknown to the compiler due to indirection layers. > > Now, the intent of the code is more clear (I probably didn't even need > to add a comment -- that's how clear it is). > > Signed-off-by: Justin Stitt <justinstitt@google.com> > --- > drivers/scsi/scsi_devinfo.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c > index 3fcaf10a9dfe..2d3dbce25629 100644 > --- a/drivers/scsi/scsi_devinfo.c > +++ b/drivers/scsi/scsi_devinfo.c > @@ -293,14 +293,16 @@ static void scsi_strcpy_devinfo(char *name, char *to, size_t to_length, > size_t from_length; > > from_length = strlen(from); > - /* This zero-pads the destination */ > - strncpy(to, from, to_length); A rare case of the padding intent being expressed! :) > - if (from_length < to_length && !compatible) { > - /* > - * space pad the string if it is short. > - */ > - memset(&to[from_length], ' ', to_length - from_length); > - } > + > + /* > + * null pad and null terminate if compatible > + * otherwise space pad > + */ > + if (compatible) > + strscpy_pad(to, from, to_length); > + else > + memcpy_and_pad(to, to_length, from, from_length, ' '); Yeah, this is much nicer to read. Reviewed-by: Kees Cook <keescook@chromium.org> (Some day I want to rename "memcpy_and_pad" ... the "and" seems verbose...)
diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c index 3fcaf10a9dfe..2d3dbce25629 100644 --- a/drivers/scsi/scsi_devinfo.c +++ b/drivers/scsi/scsi_devinfo.c @@ -293,14 +293,16 @@ static void scsi_strcpy_devinfo(char *name, char *to, size_t to_length, size_t from_length; from_length = strlen(from); - /* This zero-pads the destination */ - strncpy(to, from, to_length); - if (from_length < to_length && !compatible) { - /* - * space pad the string if it is short. - */ - memset(&to[from_length], ' ', to_length - from_length); - } + + /* + * null pad and null terminate if compatible + * otherwise space pad + */ + if (compatible) + strscpy_pad(to, from, to_length); + else + memcpy_and_pad(to, to_length, from, from_length, ' '); + if (from_length > to_length) printk(KERN_WARNING "%s: %s string '%s' is too long\n", __func__, name, from);
Depending on the state of @compatible, we are going to do different things with our @to buffer. When @compatible is true we want a NUL-term'd and NUL-padded destination buffer. Conversely, if @compatible is false we just want a space-padded destination buffer (no NUL-term required). As per: /** * scsi_dev_info_list_add_keyed - add one dev_info list entry. * @compatible: if true, null terminate short strings. Otherwise space pad. ... Note that we can't easily use `strtomem_pad` here as the size of the @to buffer is unknown to the compiler due to indirection layers. Now, the intent of the code is more clear (I probably didn't even need to add a comment -- that's how clear it is). Signed-off-by: Justin Stitt <justinstitt@google.com> --- drivers/scsi/scsi_devinfo.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)