[bug report] drm/amd/display: use FB pitch to fill dc_cursor_attributes

From: Dan Carpenter <dan.carpenter@oracle.com>
To: contact@emersion.fr
Cc: amd-gfx@lists.freedesktop.org
Subject: [bug report] drm/amd/display: use FB pitch to fill dc_cursor_attributes
Date: Mon, 7 Dec 2020 17:51:29 +0300	[thread overview]
Message-ID: <X85BcS43BIlg6GRx@mwanda> (raw)

Hello Simon Ser,

This is a semi-automatic email about new static checker warnings.

The patch 03a663673063: "drm/amd/display: use FB pitch to fill 
dc_cursor_attributes" from Dec 2, 2020, leads to the following Smatch 
complaint:

    drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:7438 handle_cursor_update()
    error: we previously assumed 'afb' could be null (see line 7397)

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
  7389  static void handle_cursor_update(struct drm_plane *plane,
  7390                                   struct drm_plane_state *old_plane_state)
  7391  {
  7392          struct amdgpu_device *adev = drm_to_adev(plane->dev);
  7393          struct amdgpu_framebuffer *afb = to_amdgpu_framebuffer(plane->state->fb);
                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
If "plane->state->fb" is NULL then "afb" is NULL.

  7394          struct drm_crtc *crtc = afb ? plane->state->crtc : old_plane_state->crtc;
                                        ^^^
Checked for NULL.

  7395          struct dm_crtc_state *crtc_state = crtc ? to_dm_crtc_state(crtc->state) : NULL;
  7396		struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
  7397		uint64_t address = afb ? afb->address : 0;
  7398		struct dc_cursor_position position;
  7399		struct dc_cursor_attributes attributes;
  7400		int ret;
  7401	
  7402		if (!plane->state->fb && !old_plane_state->fb)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
These aren't allow to be both NULL

  7403			return;
  7404	
  7405		DRM_DEBUG_DRIVER("%s: crtc_id=%d with size %d to %d\n",
  7406				 __func__,
  7407				 amdgpu_crtc->crtc_id,
  7408				 plane->state->crtc_w,
  7409				 plane->state->crtc_h);
  7410	
  7411		ret = get_cursor_position(plane, crtc, &position);
  7412		if (ret)
  7413			return;
  7414	
  7415		if (!position.enable) {
  7416			/* turn off cursor */
  7417			if (crtc_state && crtc_state->stream) {
  7418				mutex_lock(&adev->dm.dc_lock);
  7419				dc_stream_set_cursor_position(crtc_state->stream,
  7420							      &position);
  7421				mutex_unlock(&adev->dm.dc_lock);
  7422			}
  7423			return;
  7424		}
  7425	
  7426		amdgpu_crtc->cursor_width = plane->state->crtc_w;
  7427		amdgpu_crtc->cursor_height = plane->state->crtc_h;
  7428	
  7429		memset(&attributes, 0, sizeof(attributes));
  7430		attributes.address.high_part = upper_32_bits(address);
  7431		attributes.address.low_part  = lower_32_bits(address);
  7432		attributes.width             = plane->state->crtc_w;
  7433		attributes.height            = plane->state->crtc_h;
  7434		attributes.color_format      = CURSOR_MODE_COLOR_PRE_MULTIPLIED_ALPHA;
  7435		attributes.rotation_angle    = 0;
  7436		attributes.attribute_flags.value = 0;
  7437	
  7438		attributes.pitch = afb->base.pitches[0] / afb->base.format->cpp[0];
                                   ^^^^^                  ^^^^^
The patch adds some new unchecked dereferences.

  7439	
  7440		if (crtc_state->stream) {

regards,
dan carpenter
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx