[AMD Official Use Only - AMD Internal Distribution Only]
Hi Srinivasan,
The expectation is that (buf_size >= target_size) check should short circuit
all cases of (buf_pos >= buf_size), so adding (buf_pos < buf_size) inside the
if statement is redundant.
However, it seems like the compiler believes that there are still cases where
(buf_pos >= buf_size) can happen. This seems to point out an incorrect
target_size calculation.
If this is the case, can you review target_size formula can provide your
assessment the reason that compiler emits this error?
Thanks,
Wenjing
-----Original Message-----
From: SHANMUGAM, SRINIVASAN <srinivasan.shanmu...@amd.com>
Sent: Tuesday, July 2, 2024 7:43 AM
To: Siqueira, Rodrigo <rodrigo.sique...@amd.com>; Pillai, Aurabindo
<aurabindo.pil...@amd.com>
Cc: amd-gfx@lists.freedesktop.org; SHANMUGAM, SRINIVASAN
<srinivasan.shanmu...@amd.com>; Liu, Wenjing <wenjing....@amd.com>; Chung,
ChiaHsuan (Tom) <chiahsuan.ch...@amd.com>; Li, Roman <roman...@amd.com>; Wu,
Hersen <hersenxs...@amd.com>; Hung, Alex <alex.h...@amd.com>; Wentland, Harry
<harry.wentl...@amd.com>
Subject: [PATCH] drm/amd/display: Add checks to prevent buffer overflow in
mod_hdcp_dump_binary_message
This commit addresses a buffer overflow issue in the
mod_hdcp_dump_binary_message function in the display/hdcp module
The issue arises when the 'buf' pointer is NULL or the 'buf_pos' index exceeds
the 'buf_size', and is passed to the sprintf function, which attempts to write
to an invalid memory location. This is leading to undefined behavior
To resolve this, checks are added to ensure that 'buf' is not NULL and
'buf_pos' is within the bounds of 'buf_size' before it is passed to the sprintf
function. This change prevents the buffer overflow.
Fixes the below:
In function ‘mod_hdcp_dump_binary_message’,
inlined from ‘mod_hdcp_dump_binary_message’ at
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:29:6,
inlined from ‘mod_hdcp_log_ddc_trace’ at
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:107:3:
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:47:25: warning:
null destination pointer [-Wformat-overflow=]
47 | sprintf(&buf[buf_pos], "%02X ", msg[i]);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘mod_hdcp_dump_binary_message’,
inlined from ‘mod_hdcp_dump_binary_message’ at
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:29:6,
inlined from ‘mod_hdcp_log_ddc_trace’ at
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:122:3:
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:47:25: warning:
null destination pointer [-Wformat-overflow=]
47 | sprintf(&buf[buf_pos], "%02X ", msg[i]);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘mod_hdcp_dump_binary_message’,
inlined from ‘mod_hdcp_dump_binary_message’ at
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:29:6,
inlined from ‘mod_hdcp_log_ddc_trace’ at
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:61:3:
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:47:25: warning:
null destination pointer [-Wformat-overflow=]
47 | sprintf(&buf[buf_pos], "%02X ", msg[i]);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘mod_hdcp_dump_binary_message’,
inlined from ‘mod_hdcp_dump_binary_message’ at
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:29:6,
inlined from ‘mod_hdcp_log_ddc_trace’ at
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:70:3:
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:47:25: warning:
null destination pointer [-Wformat-overflow=]
47 | sprintf(&buf[buf_pos], "%02X ", msg[i]);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘mod_hdcp_dump_binary_message’,
inlined from ‘mod_hdcp_dump_binary_message’ at
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:29:6,
inlined from ‘mod_hdcp_log_ddc_trace’ at
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:73:3:
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:47:25: warning:
null destination pointer [-Wformat-overflow=]
47 | sprintf(&buf[buf_pos], "%02X ", msg[i]);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘mod_hdcp_dump_binary_message’,
inlined from ‘mod_hdcp_dump_binary_message’ at
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:29:6,
inlined from ‘mod_hdcp_log_ddc_trace’ at
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:70:3:
drivers/gpu/drm/amd/amdgpu/../display/modules/hdcp/hdcp_log.c:47:25: warning:
null destination pointer [-Wformat-overflow=]
47 | sprintf(&buf[buf_pos], "%02X ", msg[i]);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Cc: Wenjing Liu <wenjing....@amd.com>
Cc: Tom Chung <chiahsuan.ch...@amd.com>
Cc: Rodrigo Siqueira <rodrigo.sique...@amd.com>
Cc: Roman Li <roman...@amd.com>
Cc: Hersen Wu <hersenxs...@amd.com>
Cc: Alex Hung <alex.h...@amd.com>
Cc: Aurabindo Pillai <aurabindo.pil...@amd.com>
Cc: Harry Wentland <harry.wentl...@amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmu...@amd.com>
---
drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c
b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c
index 6b3b5f610907..285251711a2d 100644
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c
@@ -40,9 +40,9 @@ void mod_hdcp_dump_binary_message(uint8_t *msg, uint32_t
msg_size,
uint32_t buf_pos = 0;
uint32_t i = 0;
- if (buf_size >= target_size) {
+ if (buf_size >= target_size && buf) {
for (i = 0; i < msg_size; i++) {
- if (i % bytes_per_line == 0)
+ if (i % bytes_per_line == 0 && buf_pos < buf_size)
buf[buf_pos++] = '\n';
sprintf(&buf[buf_pos], "%02X ", msg[i]);
buf_pos += byte_size;
--
2.34.1
