| Message ID | 20240320-strncpy-drivers-video-fbdev-uvesafb-c-v1-1-fd6af3766c80@google.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | fbdev: uvesafb: replace deprecated strncpy with strscpy_pad | expand |
On Wed, Mar 20, 2024 at 11:04:44PM +0000, Justin Stitt wrote: > 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 v86d_path to be NUL-terminated based on its use with the > C-string format specifier in printf-likes: > | pr_err("failed to execute %s\n", v86d_path); > and > | return snprintf(buf, PAGE_SIZE, "%s\n", v86d_path); > > Let's also opt to pad v86d_path since it may get used in and around > userspace: > | return call_usermodehelper(v86d_path, argv, envp, UMH_WAIT_PROC); > > Considering the above, strscpy_pad() is the best replacement as it > guarantees both NUL-termination and NUL-padding on the destination > buffer. > > Note that this patch relies on the _new_ 2-argument versions of > strscpy() and strscpy_pad() introduced in Commit e6584c3964f2f ("string: > Allow 2-argument strscpy()"). > > 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 > Link: https://github.com/KSPP/linux/issues/90 > Cc: linux-hardening@vger.kernel.org > Signed-off-by: Justin Stitt <justinstitt@google.com> Reviewed-by: Kees Cook <keescook@chromium.org>
diff --git a/drivers/video/fbdev/uvesafb.c b/drivers/video/fbdev/uvesafb.c index e1f421e91b4f..8b554fcf1bfb 100644 --- a/drivers/video/fbdev/uvesafb.c +++ b/drivers/video/fbdev/uvesafb.c @@ -1867,7 +1867,7 @@ static ssize_t v86d_show(struct device_driver *dev, char *buf) static ssize_t v86d_store(struct device_driver *dev, const char *buf, size_t count) { - strncpy(v86d_path, buf, PATH_MAX - 1); + strscpy_pad(v86d_path, buf); return count; } static DRIVER_ATTR_RW(v86d);
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 v86d_path to be NUL-terminated based on its use with the C-string format specifier in printf-likes: | pr_err("failed to execute %s\n", v86d_path); and | return snprintf(buf, PAGE_SIZE, "%s\n", v86d_path); Let's also opt to pad v86d_path since it may get used in and around userspace: | return call_usermodehelper(v86d_path, argv, envp, UMH_WAIT_PROC); Considering the above, strscpy_pad() is the best replacement as it guarantees both NUL-termination and NUL-padding on the destination buffer. Note that this patch relies on the _new_ 2-argument versions of strscpy() and strscpy_pad() introduced in Commit e6584c3964f2f ("string: Allow 2-argument strscpy()"). 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 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/video/fbdev/uvesafb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) --- base-commit: a4145ce1e7bc247fd6f2846e8699473448717b37 change-id: 20240320-strncpy-drivers-video-fbdev-uvesafb-c-43668c4ef6c8 Best regards, -- Justin Stitt <justinstitt@google.com>