Re: [PATCH 09/19] drm/amd/display/amdgpu_dm: Use DC accessors in debugfs paths

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] drm/amd/display: use-after-free in dp_dsc_clock_en_write()
  [1]
--

commit 8d9d786f748dd7764b2335ea8779ba852b1d97b2
Author: Dave Airlie <airlied@redhat.com>

drm/amd/display/amdgpu_dm: Use DC accessors in debugfs paths

This commit replaces direct struct dc member dereferences in
amdgpu_dm_debugfs.c with DC accessor functions and adds a setter for the
eDP hotplug detection mode.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> index 68356cb8e4dec..fddb47a32fb9e 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
[ ... ]
> @@ -1696,7 +1696,7 @@ static ssize_t dp_dsc_clock_en_write(struct file *f, const char __user *buf,
>  	}
>  
>  	for (i = 0; i < MAX_PIPES; i++) {
> -		pipe_ctx = &aconnector->dc_link->dc->current_state->res_ctx.pipe_ctx[i];
> +		pipe_ctx = &dc_get_current_state(aconnector->dc_link->dc)->res_ctx.pipe_ctx[i];
>  		if (pipe_ctx->stream &&
>  		    pipe_ctx->stream->link == aconnector->dc_link &&
>  		    pipe_ctx->stream->sink &&

[Severity: Critical]
This problem wasn't introduced by this patch, but can a concurrent atomic
commit free current_state while dp_dsc_clock_en_write() iterates over
res_ctx.pipe_ctx[] [1]?

dp_dsc_clock_en_write() walks dc_get_current_state()->res_ctx.pipe_ctx[]
before taking dev->mode_config.mutex and dev->mode_config.connection_mutex:

drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c:dp_dsc_clock_en_write() {
	...
	for (i = 0; i < MAX_PIPES; i++) {
		pipe_ctx = &dc_get_current_state(aconnector->dc_link->dc)->res_ctx.pipe_ctx[i];
		if (pipe_ctx->stream &&
		    ...
			break;
	}

	if (!pipe_ctx->stream)
		goto done;

	// Get CRTC state
	mutex_lock(&dev->mode_config.mutex);
	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
	...
}

CPU0                                  CPU1
dp_dsc_clock_en_write()
  state = dc_get_current_state(dc)
                                      amdgpu_dm_atomic_commit_tail()
                                        ...
                                        dc_state_release(old_state)
  pipe_ctx = &state->res_ctx.pipe_ctx[i]
  if (pipe_ctx->stream && ...)        <-- use-after-free

If an atomic modeset commit swaps out and frees current_state during the
unlocked loop, dp_dsc_clock_en_write() dereferences freed memory.

[1] https://sashiko.dev/#/bug/linux-3a852cda-9b25-49c2-b119-47f2a71a94c3

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-amdgpu-dc-cleanup-v1-0-0fae41718ad2@redhat.com?part=9