[PATCH v2] drm/amd/display: Fix NULL pointer dereferences in dm_update_crtc_state() v2

[Pillai, Aurabindo]

[AMD Official Use Only - AMD Internal Distribution Only]

Reviewed-by: Aurabindo Pillai <aurabindo.pillai at amd.com>

--

Regards,
Aurabindo Pillai
________________________________
From: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM at amd.com>
Sent: Monday, April 21, 2025 12:06 AM
To: Pillai, Aurabindo <Aurabindo.Pillai at amd.com>
Cc: amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>; SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM at amd.com>; Wentland, Harry <Harry.Wentland at amd.com>; Kazlauskas, Nicholas <Nicholas.Kazlauskas at amd.com>; Chung, ChiaHsuan (Tom) <ChiaHsuan.Chung at amd.com>; Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>; Li, Roman <Roman.Li at amd.com>; Hung, Alex <Alex.Hung at amd.com>; Dan Carpenter <dan.carpenter at linaro.org>
Subject: [PATCH v2] drm/amd/display: Fix NULL pointer dereferences in dm_update_crtc_state() v2

Added checks for NULL values after retrieving drm_new_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

v2: Modified the NULL pointer check for drm_new_conn_state in the
    dm_update_crtc_state function to  include a warning via WARN_ON and
    return -EINVAL to indicate an invalid state when the pointer is NULL.

Cc: Harry Wentland <harry.wentland at amd.com>
Cc: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
Cc: Tom Chung <chiahsuan.chung at amd.com>
Cc: Rodrigo Siqueira <Rodrigo.Siqueira at amd.com>
Cc: Roman Li <roman.li at amd.com>
Cc: Alex Hung <alex.hung at amd.com>
Cc: Aurabindo Pillai <aurabindo.pillai at amd.com>
Reported-by: Dan Carpenter <dan.carpenter at linaro.org>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
Reviewed-by: Aurabindo Pillai <aurabindo.pillai at amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 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 31a5b8fc4dc4..3d2ff5d58067 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -10858,8 +10858,8 @@ 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);
+               if (WARN_ON(!drm_new_conn_state)) {
+                       ret = -EINVAL;
                         goto fail;
                 }

--
2.34.1

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20250421/5c916947/attachment.htm>