From: Justin Stitt <justinstitt@google.com>
To: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: linux-scsi@vger.kernel.org, target-devel@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org,
Justin Stitt <justinstitt@google.com>
Subject: [PATCH] scsi: target: replace deprecated strncpy with strscpy
Date: Mon, 18 Mar 2024 21:32:01 +0000 [thread overview]
Message-ID: <20240318-strncpy-drivers-target-target_core_configfs-c-v1-1-dc319e85fe45@google.com> (raw)
strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.
We expect db_root and db_root_stage to be NUL-terminated based on its
immediate use with pr_debug which expects a C-string argument (%s).
Moreover, it seems NUL-padding is not required.
Considering the above, a suitable replacement is `strscpy` [2] due to
the fact that it guarantees NUL-termination on the destination buffer
without unnecessarily NUL-padding.
Additionally, we should also change snprintf() to scnprintf().
`read_bytes` may be improperly assigned as snprintf() does NOT return
the number of bytes written into the destination buffer, rather it
returns the number of bytes that COULD have been written to that buffer
if it had ample space. Conversely, scnprintf() returns the actual number
of bytes written into the destination buffer (except the NUL-byte). This
essentially means the ``if (!read_bytes)`` was probably never a possible
branch.
After these changes, this code is more self-describing since it uses
string APIs that more accurately match the desired behavior.
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Note: build-tested only.
Found with: $ rg "strncpy\("
---
drivers/target/target_core_configfs.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/target/target_core_configfs.c b/drivers/target/target_core_configfs.c
index c1fbcdd16182..d78647e4f6c5 100644
--- a/drivers/target/target_core_configfs.c
+++ b/drivers/target/target_core_configfs.c
@@ -123,7 +123,7 @@ static ssize_t target_core_item_dbroot_store(struct config_item *item,
goto unlock;
}
- read_bytes = snprintf(db_root_stage, DB_ROOT_LEN, "%s", page);
+ read_bytes = scnprintf(db_root_stage, DB_ROOT_LEN, "%s", page);
if (!read_bytes)
goto unlock;
@@ -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, sizeof(db_root));
pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
r = read_bytes;
@@ -3651,7 +3651,7 @@ static void target_init_dbroot(void)
{
struct file *fp;
- snprintf(db_root_stage, DB_ROOT_LEN, DB_ROOT_PREFERRED);
+ scnprintf(db_root_stage, DB_ROOT_LEN, DB_ROOT_PREFERRED);
fp = filp_open(db_root_stage, O_RDONLY, 0);
if (IS_ERR(fp)) {
pr_err("db_root: cannot open: %s\n", db_root_stage);
@@ -3664,7 +3664,7 @@ static void target_init_dbroot(void)
}
filp_close(fp, NULL);
- strncpy(db_root, db_root_stage, DB_ROOT_LEN);
+ strscpy(db_root, db_root_stage, sizeof(db_root));
pr_debug("Target_Core_ConfigFS: db_root set to %s\n", db_root);
}
---
base-commit: bf3a69c6861ff4dc7892d895c87074af7bc1c400
change-id: 20240315-strncpy-drivers-target-target_core_configfs-c-fbe862bf950a
Best regards,
--
Justin Stitt <justinstitt@google.com>
next reply other threads:[~2024-03-18 21:32 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-18 21:32 Justin Stitt [this message]
2024-03-18 21:42 ` [PATCH] scsi: target: replace deprecated strncpy with strscpy Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240318-strncpy-drivers-target-target_core_configfs-c-v1-1-dc319e85fe45@google.com \
--to=justinstitt@google.com \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=target-devel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).