* [PATCH] drm/amd/display: fix NULL ptr deref in ISM delayed work
@ 2026-04-09 7:20 Ray Wu
2026-04-09 15:05 ` Leo Li
2026-04-09 18:15 ` Matthew Schwartz
0 siblings, 2 replies; 3+ messages in thread
From: Ray Wu @ 2026-04-09 7:20 UTC (permalink / raw)
To: amd-gfx
Cc: Harry Wentland, Leo Li, Aurabindo Pillai, Roman Li, Wayne Lin,
Tom Chung, Fangzhi Zuo, Dan Wheeler, Ray Wu, Ivan Lipski,
Alex Hung, Ray Wu
dc_destroy() sets dm->dc to NULL before amdgpu_dm_ism_fini() is called,
leaving a window where in-flight ISM delayed work dereferences the stale
pointer. Call amdgpu_dm_ism_fini() in amdgpu_dm_fini() before dc_destroy().
Fixes: f5d0d3f3439e ("drm/amd/display: Add Idle state manager(ISM)")
Signed-off-by: Ray Wu <ray.wu@amd.com>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 9 +++++++++
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 7 ++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
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 bac02ea15b8a..bb79b6bed3c4 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2236,6 +2236,8 @@ static int amdgpu_dm_early_fini(struct amdgpu_ip_block *ip_block)
static void amdgpu_dm_fini(struct amdgpu_device *adev)
{
int i;
+ struct drm_crtc *crtc;
+ struct amdgpu_crtc *acrtc;
if (adev->dm.vblank_control_workqueue) {
destroy_workqueue(adev->dm.vblank_control_workqueue);
@@ -2252,6 +2254,13 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
adev->dm.idle_workqueue = NULL;
}
+ /* Finalize ISM for each CRTC before dc_destroy() sets dm->dc to NULL */
+ drm_for_each_crtc(crtc, adev_to_drm(adev)) {
+ acrtc = to_amdgpu_crtc(crtc);
+ amdgpu_dm_ism_fini(&acrtc->ism);
+
+ }
+
amdgpu_dm_destroy_drm_device(&adev->dm);
#if defined(CONFIG_DRM_AMD_SECURE_DISPLAY)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
index 26f3d513576b..de203445e084 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
@@ -459,7 +459,12 @@ static void amdgpu_dm_crtc_destroy(struct drm_crtc *crtc)
{
struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
- amdgpu_dm_ism_fini(&acrtc->ism);
+ /*
+ * amdgpu_dm_ism_fini() is intentionally called in amdgpu_dm_fini().
+ * It must be called before dc_destroy() in amdgpu_dm_fini()
+ * to avoid ISM accessing an invalid dc handle once dc is released.
+ */
+
drm_crtc_cleanup(crtc);
kfree(crtc);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] drm/amd/display: fix NULL ptr deref in ISM delayed work
2026-04-09 7:20 [PATCH] drm/amd/display: fix NULL ptr deref in ISM delayed work Ray Wu
@ 2026-04-09 15:05 ` Leo Li
2026-04-09 18:15 ` Matthew Schwartz
1 sibling, 0 replies; 3+ messages in thread
From: Leo Li @ 2026-04-09 15:05 UTC (permalink / raw)
To: Ray Wu, amd-gfx
Cc: Harry Wentland, Aurabindo Pillai, Roman Li, Wayne Lin, Tom Chung,
Fangzhi Zuo, Dan Wheeler, Ivan Lipski, Alex Hung
On 2026-04-09 03:20, Ray Wu wrote:
> dc_destroy() sets dm->dc to NULL before amdgpu_dm_ism_fini() is called,
> leaving a window where in-flight ISM delayed work dereferences the stale
> pointer. Call amdgpu_dm_ism_fini() in amdgpu_dm_fini() before dc_destroy().
>
> Fixes: f5d0d3f3439e ("drm/amd/display: Add Idle state manager(ISM)")
> Signed-off-by: Ray Wu <ray.wu@amd.com>
Reviewed-by: Leo Li <sunpeng.li@amd.com>
Thanks!
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 9 +++++++++
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 7 ++++++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> 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 bac02ea15b8a..bb79b6bed3c4 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2236,6 +2236,8 @@ static int amdgpu_dm_early_fini(struct amdgpu_ip_block *ip_block)
> static void amdgpu_dm_fini(struct amdgpu_device *adev)
> {
> int i;
> + struct drm_crtc *crtc;
> + struct amdgpu_crtc *acrtc;
>
> if (adev->dm.vblank_control_workqueue) {
> destroy_workqueue(adev->dm.vblank_control_workqueue);
> @@ -2252,6 +2254,13 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
> adev->dm.idle_workqueue = NULL;
> }
>
> + /* Finalize ISM for each CRTC before dc_destroy() sets dm->dc to NULL */
> + drm_for_each_crtc(crtc, adev_to_drm(adev)) {
> + acrtc = to_amdgpu_crtc(crtc);
> + amdgpu_dm_ism_fini(&acrtc->ism);
> +
> + }
> +
> amdgpu_dm_destroy_drm_device(&adev->dm);
>
> #if defined(CONFIG_DRM_AMD_SECURE_DISPLAY)
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> index 26f3d513576b..de203445e084 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> @@ -459,7 +459,12 @@ static void amdgpu_dm_crtc_destroy(struct drm_crtc *crtc)
> {
> struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
>
> - amdgpu_dm_ism_fini(&acrtc->ism);
> + /*
> + * amdgpu_dm_ism_fini() is intentionally called in amdgpu_dm_fini().
> + * It must be called before dc_destroy() in amdgpu_dm_fini()
> + * to avoid ISM accessing an invalid dc handle once dc is released.
> + */
> +
> drm_crtc_cleanup(crtc);
> kfree(crtc);
> }
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] drm/amd/display: fix NULL ptr deref in ISM delayed work
2026-04-09 7:20 [PATCH] drm/amd/display: fix NULL ptr deref in ISM delayed work Ray Wu
2026-04-09 15:05 ` Leo Li
@ 2026-04-09 18:15 ` Matthew Schwartz
1 sibling, 0 replies; 3+ messages in thread
From: Matthew Schwartz @ 2026-04-09 18:15 UTC (permalink / raw)
To: Ray Wu, amd-gfx
Cc: Harry Wentland, Leo Li, Aurabindo Pillai, Roman Li, Wayne Lin,
Tom Chung, Fangzhi Zuo, Dan Wheeler, Ivan Lipski, Alex Hung
On 4/9/26 12:20 AM, Ray Wu wrote:
> dc_destroy() sets dm->dc to NULL before amdgpu_dm_ism_fini() is called,
> leaving a window where in-flight ISM delayed work dereferences the stale
> pointer. Call amdgpu_dm_ism_fini() in amdgpu_dm_fini() before dc_destroy().
>
> Fixes: f5d0d3f3439e ("drm/amd/display: Add Idle state manager(ISM)")
> Signed-off-by: Ray Wu <ray.wu@amd.com>
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 9 +++++++++
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 7 ++++++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> 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 bac02ea15b8a..bb79b6bed3c4 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2236,6 +2236,8 @@ static int amdgpu_dm_early_fini(struct amdgpu_ip_block *ip_block)
> static void amdgpu_dm_fini(struct amdgpu_device *adev)
> {
> int i;
> + struct drm_crtc *crtc;
> + struct amdgpu_crtc *acrtc;
>
> if (adev->dm.vblank_control_workqueue) {
> destroy_workqueue(adev->dm.vblank_control_workqueue);
> @@ -2252,6 +2254,13 @@ static void amdgpu_dm_fini(struct amdgpu_device *adev)
> adev->dm.idle_workqueue = NULL;
> }
>
> + /* Finalize ISM for each CRTC before dc_destroy() sets dm->dc to NULL */
> + drm_for_each_crtc(crtc, adev_to_drm(adev)) {
> + acrtc = to_amdgpu_crtc(crtc);
> + amdgpu_dm_ism_fini(&acrtc->ism);
> +
> + }
> +
> amdgpu_dm_destroy_drm_device(&adev->dm);
>
> #if defined(CONFIG_DRM_AMD_SECURE_DISPLAY)
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> index 26f3d513576b..de203445e084 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> @@ -459,7 +459,12 @@ static void amdgpu_dm_crtc_destroy(struct drm_crtc *crtc)
> {
> struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
>
> - amdgpu_dm_ism_fini(&acrtc->ism);
> + /*
> + * amdgpu_dm_ism_fini() is intentionally called in amdgpu_dm_fini().
> + * It must be called before dc_destroy() in amdgpu_dm_fini()
> + * to avoid ISM accessing an invalid dc handle once dc is released.
> + */
I'm seeing a new build warning with this hunk:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_crtc.c: In function ‘amdgpu_dm_crtc_destroy’:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_crtc.c:460:29: warning: unused variable ‘acrtc’ [-Wunused-variable]
460 | struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
| ^~~~~
This diff resolved it:
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
index de203445e0844..40c5f74dbe2b6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
@@ -457,8 +457,6 @@ static struct drm_crtc_state *amdgpu_dm_crtc_duplicate_state(struct drm_crtc *cr
static void amdgpu_dm_crtc_destroy(struct drm_crtc *crtc)
{
- struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
-
/*
* amdgpu_dm_ism_fini() is intentionally called in amdgpu_dm_fini().
* It must be called before dc_destroy() in amdgpu_dm_fini()
Thanks,
Matt
> +
> drm_crtc_cleanup(crtc);
> kfree(crtc);
> }
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-10 7:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-09 7:20 [PATCH] drm/amd/display: fix NULL ptr deref in ISM delayed work Ray Wu
2026-04-09 15:05 ` Leo Li
2026-04-09 18:15 ` Matthew Schwartz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox