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 <[email protected]>
---
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
