This commit adds a null check for the 'afb' variable in the
amdgpu_dm_update_cursor function. Previously, 'afb' was assumed to be
null at line 8388, but was used later in the code without a null check.
This could potentially lead to a null pointer dereference.

Fixes the below:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:8433 
amdgpu_dm_update_cursor()
        error: we previously assumed 'afb' could be null (see line 8388)

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
    8379 static void amdgpu_dm_update_cursor(struct drm_plane *plane,
    8380                                     struct drm_plane_state 
*old_plane_state,
    8381                                     struct dc_stream_update *update)
    8382 {
    8383         struct amdgpu_device *adev = drm_to_adev(plane->dev);
    8384         struct amdgpu_framebuffer *afb = 
to_amdgpu_framebuffer(plane->state->fb);
    8385         struct drm_crtc *crtc = afb ? plane->state->crtc : 
old_plane_state->crtc;
                                         ^^^^^

    8386         struct dm_crtc_state *crtc_state = crtc ? 
to_dm_crtc_state(crtc->state) : NULL;
    8387         struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
    8388         uint64_t address = afb ? afb->address : 0;
                                    ^^^^^ Checks for NULL

    8389         struct dc_cursor_position position = {0};
    8390         struct dc_cursor_attributes attributes;
    8391         int ret;
    8392
    8393         if (!plane->state->fb && !old_plane_state->fb)
    8394                 return;
    8395
    8396         drm_dbg_atomic(plane->dev, "crtc_id=%d with size %d to %d\n",
    8397                        amdgpu_crtc->crtc_id, plane->state->crtc_w,
    8398                        plane->state->crtc_h);
    8399
    8400         ret = amdgpu_dm_plane_get_cursor_position(plane, crtc, 
&position);
    8401         if (ret)
    8402                 return;
    8403
    8404         if (!position.enable) {
    8405                 /* turn off cursor */
    8406                 if (crtc_state && crtc_state->stream) {
    8407                         
dc_stream_set_cursor_position(crtc_state->stream,
    8408                                                       &position);
    8409                         update->cursor_position = 
&crtc_state->stream->cursor_position;
    8410                 }
    8411                 return;
    8412         }
    8413
    8414         amdgpu_crtc->cursor_width = plane->state->crtc_w;
    8415         amdgpu_crtc->cursor_height = plane->state->crtc_h;
    8416
    8417         memset(&attributes, 0, sizeof(attributes));
    8418         attributes.address.high_part = upper_32_bits(address);
    8419         attributes.address.low_part  = lower_32_bits(address);
    8420         attributes.width             = plane->state->crtc_w;
    8421         attributes.height            = plane->state->crtc_h;
    8422         attributes.color_format      = 
CURSOR_MODE_COLOR_PRE_MULTIPLIED_ALPHA;
    8423         attributes.rotation_angle    = 0;
    8424         attributes.attribute_flags.value = 0;
    8425
    8426         /* Enable cursor degamma ROM on DCN3+ for implicit sRGB 
degamma in DRM
    8427          * legacy gamma setup.
    8428          */
    8429         if (crtc_state->cm_is_degamma_srgb &&
    8430             adev->dm.dc->caps.color.dpp.gamma_corr)
    8431                 attributes.attribute_flags.bits.ENABLE_CURSOR_DEGAMMA 
= 1;
    8432
--> 8433         attributes.pitch = afb->base.pitches[0] / 
afb->base.format->cpp[0];
                                    ^^^^^                  ^^^^^
Unchecked dereferences

    8434
    8435         if (crtc_state->stream) {
    8436                 if 
(!dc_stream_set_cursor_attributes(crtc_state->stream,
    8437                                                      &attributes))
    8438                         DRM_ERROR("DC failed to set cursor 
attributes\n");
    8439
    8440                 update->cursor_attributes = 
&crtc_state->stream->cursor_attributes;
    8441
    8442                 if (!dc_stream_set_cursor_position(crtc_state->stream,
    8443                                                    &position))
    8444                         DRM_ERROR("DC failed to set cursor 
position\n");
    8445
    8446                 update->cursor_position = 
&crtc_state->stream->cursor_position;
    8447         }
    8448 }

Fixes: 66eba12a5482 ("drm/amd/display: Do cursor programming with rest of pipe")
Reported-by: Dan Carpenter <dan.carpen...@linaro.org>
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>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c    | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 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 6196de6cebbf..6d468badb669 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8637,14 +8637,22 @@ static void amdgpu_dm_update_cursor(struct drm_plane 
*plane,
 {
        struct amdgpu_device *adev = drm_to_adev(plane->dev);
        struct amdgpu_framebuffer *afb = 
to_amdgpu_framebuffer(plane->state->fb);
-       struct drm_crtc *crtc = afb ? plane->state->crtc : 
old_plane_state->crtc;
-       struct dm_crtc_state *crtc_state = crtc ? to_dm_crtc_state(crtc->state) 
: NULL;
-       struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-       uint64_t address = afb ? afb->address : 0;
+       struct drm_crtc *crtc;
+       struct dm_crtc_state *crtc_state;
+       struct amdgpu_crtc *amdgpu_crtc;
+       u64 address;
        struct dc_cursor_position position = {0};
        struct dc_cursor_attributes attributes;
        int ret;
 
+       if (!afb)
+               return;
+
+       crtc = plane->state->crtc ? plane->state->crtc : old_plane_state->crtc;
+       crtc_state = crtc ? to_dm_crtc_state(crtc->state) : NULL;
+       amdgpu_crtc = to_amdgpu_crtc(crtc);
+       address = afb->address;
+
        if (!plane->state->fb && !old_plane_state->fb)
                return;
 
-- 
2.34.1

Reply via email to