linux-kernel - [PATCH] video/hdmi: prefer length specifier in format over string copying

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240320-strncpy-drivers-video-hdmi-c-v1-1-f9a08168cdaf@google.com>
Date: Wed, 20 Mar 2024 23:22:02 +0000
From: Justin Stitt <justinstitt@...gle.com>
To: Helge Deller <deller@....de>
Cc: linux-fbdev@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org, 
	Justin Stitt <justinstitt@...gle.com>
Subject: [PATCH] video/hdmi: prefer length specifier in format over string copying

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@...r.kernel.org
Signed-off-by: Justin Stitt <justinstitt@...gle.com>
---
Note: build-tested only.

Found with: $ rg "strncpy\("
---
 drivers/video/hdmi.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

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);
 }

---
base-commit: a4145ce1e7bc247fd6f2846e8699473448717b37
change-id: 20240320-strncpy-drivers-video-hdmi-c-bc18d585971f

Best regards,
--
Justin Stitt <justinstitt@...gle.com>