<!DOCTYPE html><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<p>The original code will check the drm_new_conn_state if it's valid
in here</p>
<pre wrap="" class="moz-quote-pre">10712 if (IS_ERR(drm_new_conn_state)) {</pre>
<p>After that the drm_new_conn_state does not touch by anyone before
call the <br>
</p>
<pre wrap="" class="moz-quote-pre">--> 10751 ret = fill_hdr_info_packet(drm_new_conn_state,</pre>
<p></p>
<p>I think it should be no issue in this case.</p>
<p>We call the <span style="white-space: pre-wrap">PTR_ERR_OR_ZERO() just because we need to get the error code and return to the caller.</span></p>
<pre wrap="" class="moz-quote-pre"> 10713 ret = PTR_ERR_OR_ZERO(drm_new_conn_state);
</pre>
<p>Maybe it's just a false warning?<br>
</p>
<p>Tom<br>
</p>
<p></p>
<div class="moz-cite-prefix">On 3/12/2025 10:34 AM, Srinivasan
Shanmugam wrote:<br>
</div>
<blockquote type="cite" cite="mid:20250312023409.2687233-1-srinivasan.shanmugam@amd.com">
<pre wrap="" class="moz-quote-pre">Added checks for NULL values after retrieving drm_new_conn_state and
drm_old_conn_state to prevent dereferencing NULL pointers.
Fixes the below:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:10751 dm_update_crtc_state()
warn: 'drm_new_conn_state' can also be NULL
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
10672 static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
10673 struct drm_atomic_state *state,
10674 struct drm_crtc *crtc,
10675 struct drm_crtc_state *old_crtc_state,
10676 struct drm_crtc_state *new_crtc_state,
10677 bool enable,
10678 bool *lock_and_validation_needed)
10679 {
10680 struct dm_atomic_state *dm_state = NULL;
10681 struct dm_crtc_state *dm_old_crtc_state, *dm_new_crtc_state;
10682 struct dc_stream_state *new_stream;
10683 int ret = 0;
10684
...
10703
10704 /* TODO This hack should go away */
10705 if (connector && enable) {
10706 /* Make sure fake sink is created in plug-in scenario */
10707 drm_new_conn_state = drm_atomic_get_new_connector_state(state,
10708 connector);
drm_atomic_get_new_connector_state() can't return error pointers, only NULL.
10709 drm_old_conn_state = drm_atomic_get_old_connector_state(state,
10710 connector);
10711
10712 if (IS_ERR(drm_new_conn_state)) {
^^^^^^^^^^^^^^^^^^
10713 ret = PTR_ERR_OR_ZERO(drm_new_conn_state);
Calling PTR_ERR_OR_ZERO() doesn't make sense. It can't be success.
10714 goto fail;
10715 }
10716
...
10748
10749 dm_new_crtc_state->abm_level = dm_new_conn_state->abm_level;
10750
--> 10751 ret = fill_hdr_info_packet(drm_new_conn_state,
^^^^^^^^^^^^^^^^^^ Unchecked dereference
10752 &new_stream->hdr_static_metadata);
10753 if (ret)
10754 goto fail;
10755
Cc: Harry Wentland <a class="moz-txt-link-rfc2396E" href="mailto:harry.wentland@amd.com"><harry.wentland@amd.com></a>
Cc: Nicholas Kazlauskas <a class="moz-txt-link-rfc2396E" href="mailto:nicholas.kazlauskas@amd.com"><nicholas.kazlauskas@amd.com></a>
Cc: Tom Chung <a class="moz-txt-link-rfc2396E" href="mailto:chiahsuan.chung@amd.com"><chiahsuan.chung@amd.com></a>
Cc: Rodrigo Siqueira <a class="moz-txt-link-rfc2396E" href="mailto:Rodrigo.Siqueira@amd.com"><Rodrigo.Siqueira@amd.com></a>
Cc: Roman Li <a class="moz-txt-link-rfc2396E" href="mailto:roman.li@amd.com"><roman.li@amd.com></a>
Cc: Alex Hung <a class="moz-txt-link-rfc2396E" href="mailto:alex.hung@amd.com"><alex.hung@amd.com></a>
Cc: Aurabindo Pillai <a class="moz-txt-link-rfc2396E" href="mailto:aurabindo.pillai@amd.com"><aurabindo.pillai@amd.com></a>
Reported-by: Dan Carpenter <a class="moz-txt-link-rfc2396E" href="mailto:dan.carpenter@linaro.org"><dan.carpenter@linaro.org></a>
Signed-off-by: Srinivasan Shanmugam <a class="moz-txt-link-rfc2396E" href="mailto:srinivasan.shanmugam@amd.com"><srinivasan.shanmugam@amd.com></a>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 20 +++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 1b92930119cc..e3df11662fff 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -10727,11 +10727,20 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
drm_old_conn_state = drm_atomic_get_old_connector_state(state,
connector);
- if (IS_ERR(drm_new_conn_state)) {
- ret = PTR_ERR_OR_ZERO(drm_new_conn_state);
- goto fail;
+ /* Check if drm_new_conn_state is valid */
+ if (drm_new_conn_state) {
+ dm_new_conn_state = to_dm_connector_state(drm_new_conn_state);
+
+ /* Attempt to fill HDR info packet */
+ ret = fill_hdr_info_packet(drm_new_conn_state,
+ &new_stream->hdr_static_metadata);
+ if (ret)
+ goto fail;
}
+ if (drm_old_conn_state)
+ dm_old_conn_state = to_dm_connector_state(drm_old_conn_state);
+
dm_new_conn_state = to_dm_connector_state(drm_new_conn_state);
dm_old_conn_state = to_dm_connector_state(drm_old_conn_state);
@@ -10766,11 +10775,6 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm,
dm_new_crtc_state->abm_level = dm_new_conn_state->abm_level;
- ret = fill_hdr_info_packet(drm_new_conn_state,
- &new_stream->hdr_static_metadata);
- if (ret)
- goto fail;
-
/*
* If we already removed the old stream from the context
* (and set the new stream to NULL) then we can't reuse
</pre>
</blockquote>
</body>
</html>