| Message ID | 20240320-strncpy-drivers-video-hdmi-c-v1-1-f9a08168cdaf@google.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | video/hdmi: prefer length specifier in format over string copying | expand |
On Wed, Mar 20, 2024 at 11:22:02PM +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. > > It looks like the main use of strncpy() here is to limit the amount of > bytes printed from hdmi_log() by using a tmp buffer and limiting the > number of bytes copied. Really, we should use the %.<len>s format > qualifier to achieve this. > > 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/hdmi.c b/drivers/video/hdmi.c index 03c7f27dde49..ba301f3f4951 100644 --- a/drivers/video/hdmi.c +++ b/drivers/video/hdmi.c @@ -1310,17 +1310,11 @@ static void hdmi_spd_infoframe_log(const char *level, struct device *dev, const struct hdmi_spd_infoframe *frame) { - u8 buf[17]; - hdmi_infoframe_log_header(level, dev, (const struct hdmi_any_infoframe *)frame); - memset(buf, 0, sizeof(buf)); - - strncpy(buf, frame->vendor, 8); - hdmi_log(" vendor: %s\n", buf); - strncpy(buf, frame->product, 16); - hdmi_log(" product: %s\n", buf); + hdmi_log(" vendor: %.8s\n", frame->vendor); + hdmi_log(" product: %.16s\n", frame->product); hdmi_log(" source device information: %s (0x%x)\n", hdmi_spd_sdi_get_name(frame->sdi), frame->sdi); }
strncpy() is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous string interfaces. It looks like the main use of strncpy() here is to limit the amount of bytes printed from hdmi_log() by using a tmp buffer and limiting the number of bytes copied. Really, we should use the %.<len>s format qualifier to achieve this. 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/hdmi.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) --- base-commit: a4145ce1e7bc247fd6f2846e8699473448717b37 change-id: 20240320-strncpy-drivers-video-hdmi-c-bc18d585971f Best regards, -- Justin Stitt <justinstitt@google.com>