| Message ID | 20250226121003.359876-1-thorsten.blum@linux.dev (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: target: Replace deprecated strncpy() with strscpy() | expand |
On Wed, Feb 26, 2025 at 01:10:03PM +0100, Thorsten Blum wrote: > Since strncpy() is deprecated for NUL-terminated destination buffers, > use strscpy() instead. > > Compile-tested only. > > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> > --- > drivers/target/target_core_configfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c > index c40217f44b1b..446682f900e4 100644 > --- a/drivers/target/target_core_configfs.c > +++ b/drivers/target/target_core_configfs.c > @@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item, > } > filp_close(fp, NULL); > > - strncpy(db_root, db_root_stage, read_bytes); > + strscpy(db_root, db_root_stage, read_bytes); > pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root); > > r = read_bytes; When doing strncpy() to strscpy() conversions, please include details of your analysis for several things: - why is it safe to be NUL-terminated? - why is NUL padding needed/not needed? - why is the maximum length argument correct? In this case, db_root is used with "%s" format strings, so we know it is expected to be NUL-terminated. Additionally, it is _only_ ever used with "5s" format strings, so padding is not needed. As for length, read_bytes will always be less than DB_ROOT_LEN because it is explicitly tested for against "count" which, enforced by the SETATTR API, will always be the number of valid bytes in "page", and snprintf() will be limited to DB_ROOT_LEN. (snprintf may return more than its length argument, but because of the "count" check, this should be impossible.) While you're in this file, though, can you replace the other strncpy that is used in target_init_dbroot() also? -Kees
On 28. Feb 2025, at 21:10, Kees Cook wrote: > On Wed, Feb 26, 2025 at 01:10:03PM +0100, Thorsten Blum wrote: >> Since strncpy() is deprecated for NUL-terminated destination buffers, >> use strscpy() instead. >> >> Compile-tested only. >> >> Link: https://github.com/KSPP/linux/issues/90 >> Cc: linux-hardening@vger.kernel.org >> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> >> --- > > When doing strncpy() to strscpy() conversions, please include details > of your analysis for several things: > > - why is it safe to be NUL-terminated? > - why is NUL padding needed/not needed? > - why is the maximum length argument correct? Yes, I'll try to describe it better. > While you're in this file, though, can you replace the other strncpy > that is used in target_init_dbroot() also? Ah, I missed this one and will submit a v2 soon. Thanks, Thorsten
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c index c40217f44b1b..446682f900e4 100644 --- a/drivers/target/target_core_configfs.c +++ b/drivers/target/target_core_configfs.c @@ -143,7 +143,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item, } filp_close(fp, NULL); - strncpy(db_root, db_root_stage, read_bytes); + strscpy(db_root, db_root_stage, read_bytes); pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root); r = read_bytes;
Since strncpy() is deprecated for NUL-terminated destination buffers, use strscpy() instead. Compile-tested only. Link: https://github.com/KSPP/linux/issues/90 Cc: linux-hardening@vger.kernel.org Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> --- drivers/target/target_core_configfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)