public inbox for kernel-janitors@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amd/display: Fix debugfs output in dp_link_settings_read()
@ 2025-12-04 13:20 Dan Carpenter
  0 siblings, 0 replies; only message in thread
From: Dan Carpenter @ 2025-12-04 13:20 UTC (permalink / raw)
  To: Hersen Wu
  Cc: Harry Wentland, Leo Li, Rodrigo Siqueira, Alex Deucher,
	Christian König, David Airlie, Simona Vetter,
	ChiaHsuan (Tom) Chung, Roman Li, Peter Shkenev,
	Timur Kristóf, Wayne Lin, Alex Hung, Kun Liu, Ray Wu,
	amd-gfx, dri-devel, linux-kernel, kernel-janitors

This code passes the wrong limit to snprintf().  It does:

	str_len = strlen("Current:  %d  0x%x  %d  ");
	snprintf(rd_buf_ptr, str_len, "....

The limit should  normally be the number of bytes remaining in the
buffer but instead of that it's using the number of bytes in the
unexpanded format string.  So if any of the numbers are more than 1
digit then the output string will have characters missing from the
middle of the output.

Normally, we would do it like this:

	off += scnprintf(p + off, buf_size - off, "...

Also we can use cleanup.h magic to free the "buf" and
simple_read_from_buffer() to copy the buffer to the user as a bit
of a cleanup.

Fixes: 41db5f1931ec ("drm/amd/display: set-read link rate and lane count through debugfs")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
Not tested.

 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 94 ++++++-------------
 1 file changed, 31 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index cb4bb67289a4..028dfd0aa43d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -185,72 +185,40 @@ static int parse_write_buffer_into_params(char *wr_buf, uint32_t wr_buf_size,
  * check current and preferred settings.
  *
  */
-static ssize_t dp_link_settings_read(struct file *f, char __user *buf,
-				 size_t size, loff_t *pos)
+static ssize_t dp_link_settings_read(struct file *f, char __user *ubuf,
+				     size_t count, loff_t *pos)
 {
 	struct amdgpu_dm_connector *connector = file_inode(f)->i_private;
 	struct dc_link *link = connector->dc_link;
-	char *rd_buf = NULL;
-	char *rd_buf_ptr = NULL;
-	const uint32_t rd_buf_size = 100;
-	uint32_t result = 0;
-	uint8_t str_len = 0;
-	int r;
-
-	if (*pos & 3 || size & 3)
-		return -EINVAL;
-
-	rd_buf = kcalloc(rd_buf_size, sizeof(char), GFP_KERNEL);
-	if (!rd_buf)
-		return 0;
-
-	rd_buf_ptr = rd_buf;
-
-	str_len = strlen("Current:  %d  0x%x  %d  ");
-	snprintf(rd_buf_ptr, str_len, "Current:  %d  0x%x  %d  ",
-			link->cur_link_settings.lane_count,
-			link->cur_link_settings.link_rate,
-			link->cur_link_settings.link_spread);
-	rd_buf_ptr += str_len;
-
-	str_len = strlen("Verified:  %d  0x%x  %d  ");
-	snprintf(rd_buf_ptr, str_len, "Verified:  %d  0x%x  %d  ",
-			link->verified_link_cap.lane_count,
-			link->verified_link_cap.link_rate,
-			link->verified_link_cap.link_spread);
-	rd_buf_ptr += str_len;
-
-	str_len = strlen("Reported:  %d  0x%x  %d  ");
-	snprintf(rd_buf_ptr, str_len, "Reported:  %d  0x%x  %d  ",
-			link->reported_link_cap.lane_count,
-			link->reported_link_cap.link_rate,
-			link->reported_link_cap.link_spread);
-	rd_buf_ptr += str_len;
-
-	str_len = strlen("Preferred:  %d  0x%x  %d  ");
-	snprintf(rd_buf_ptr, str_len, "Preferred:  %d  0x%x  %d\n",
-			link->preferred_link_setting.lane_count,
-			link->preferred_link_setting.link_rate,
-			link->preferred_link_setting.link_spread);
-
-	while (size) {
-		if (*pos >= rd_buf_size)
-			break;
-
-		r = put_user(*(rd_buf + result), buf);
-		if (r) {
-			kfree(rd_buf);
-			return r; /* r = -EFAULT */
-		}
-
-		buf += 1;
-		size -= 1;
-		*pos += 1;
-		result += 1;
-	}
-
-	kfree(rd_buf);
-	return result;
+	size_t size = 1024;
+	int off;
+
+	char *buf __free(kfree) = kcalloc(size, sizeof(char), GFP_KERNEL);
+	if (!buf)
+		return  -ENOMEM;
+
+	off = 0;
+	off += scnprintf(buf + off, size - off, "Current:  %d  0x%x  %d  ",
+			 link->cur_link_settings.lane_count,
+			 link->cur_link_settings.link_rate,
+			 link->cur_link_settings.link_spread);
+
+	off += scnprintf(buf + off, size - off, "Verified:  %d  0x%x  %d  ",
+			 link->verified_link_cap.lane_count,
+			 link->verified_link_cap.link_rate,
+			 link->verified_link_cap.link_spread);
+
+	off += scnprintf(buf + off, size - off, "Reported:  %d  0x%x  %d  ",
+			 link->reported_link_cap.lane_count,
+			 link->reported_link_cap.link_rate,
+			 link->reported_link_cap.link_spread);
+
+	off += scnprintf(buf + off, size - off, "Preferred:  %d  0x%x  %d\n",
+			 link->preferred_link_setting.lane_count,
+			 link->preferred_link_setting.link_rate,
+			 link->preferred_link_setting.link_spread);
+
+	return simple_read_from_buffer(ubuf, count, pos, buf, off);
 }
 
 static ssize_t dp_link_settings_write(struct file *f, const char __user *buf,
-- 
2.51.0


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2025-12-04 13:20 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-04 13:20 [PATCH] drm/amd/display: Fix debugfs output in dp_link_settings_read() Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox