[v1] drm/mediatek: Add error handling for old state CRTC in atomic_disable

[RESEND PATCH] drm/mediatek: Add error handling for old state CRTC in atomic_disable

Posted by Jason-JH Lin 2 months, 1 week ago

Introduce error handling to address an issue where, after a hotplug
event, the cursor continues to update. This situation can lead to a
kernel panic due to accessing the NULL `old_state->crtc`.

E,g.
Unable to handle kernel NULL pointer dereference at virtual address
Call trace:
 mtk_crtc_plane_disable+0x24/0x140
 mtk_plane_atomic_update+0x8c/0xa8
 drm_atomic_helper_commit_planes+0x114/0x2c8
 drm_atomic_helper_commit_tail_rpm+0x4c/0x158
 commit_tail+0xa0/0x168
 drm_atomic_helper_commit+0x110/0x120
 drm_atomic_commit+0x8c/0xe0
 drm_atomic_helper_update_plane+0xd4/0x128
 __setplane_atomic+0xcc/0x110
 drm_mode_cursor_common+0x250/0x440
 drm_mode_cursor_ioctl+0x44/0x70
 drm_ioctl+0x264/0x5d8
 __arm64_sys_ioctl+0xd8/0x510
 invoke_syscall+0x6c/0xe0
 do_el0_svc+0x68/0xe8
 el0_svc+0x34/0x60
 el0t_64_sync_handler+0x1c/0xf8
 el0t_64_sync+0x180/0x188

Adding NULL pointer checks to ensure stability by preventing operations
on an invalid CRTC state.

Fixes: d208261e9f7c ("drm/mediatek: Add wait_event_timeout when disabling plane")
Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
---
RESEND change:
- Update author and Signed-off-by name.
---
 drivers/gpu/drm/mediatek/mtk_plane.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_plane.c b/drivers/gpu/drm/mediatek/mtk_plane.c
index cbc4f37da8ba..02349bd44001 100644
--- a/drivers/gpu/drm/mediatek/mtk_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_plane.c
@@ -292,7 +292,8 @@ static void mtk_plane_atomic_disable(struct drm_plane *plane,
 	wmb(); /* Make sure the above parameter is set before update */
 	mtk_plane_state->pending.dirty = true;
 
-	mtk_crtc_plane_disable(old_state->crtc, plane);
+	if (old_state && old_state->crtc)
+		mtk_crtc_plane_disable(old_state->crtc, plane);
 }
 
 static void mtk_plane_atomic_update(struct drm_plane *plane,
-- 
2.43.0

Re: [RESEND PATCH] drm/mediatek: Add error handling for old state CRTC in atomic_disable

Posted by Chun-Kuang Hu 1 month, 3 weeks ago

Hi, Jason:

Jason-JH Lin <jason-jh.lin@mediatek.com> 於 2025年7月28日 週一 上午10:50寫道：
>
> Introduce error handling to address an issue where, after a hotplug
> event, the cursor continues to update. This situation can lead to a
> kernel panic due to accessing the NULL `old_state->crtc`.
>
> E,g.
> Unable to handle kernel NULL pointer dereference at virtual address
> Call trace:
>  mtk_crtc_plane_disable+0x24/0x140
>  mtk_plane_atomic_update+0x8c/0xa8
>  drm_atomic_helper_commit_planes+0x114/0x2c8
>  drm_atomic_helper_commit_tail_rpm+0x4c/0x158
>  commit_tail+0xa0/0x168
>  drm_atomic_helper_commit+0x110/0x120
>  drm_atomic_commit+0x8c/0xe0
>  drm_atomic_helper_update_plane+0xd4/0x128
>  __setplane_atomic+0xcc/0x110
>  drm_mode_cursor_common+0x250/0x440
>  drm_mode_cursor_ioctl+0x44/0x70
>  drm_ioctl+0x264/0x5d8
>  __arm64_sys_ioctl+0xd8/0x510
>  invoke_syscall+0x6c/0xe0
>  do_el0_svc+0x68/0xe8
>  el0_svc+0x34/0x60
>  el0t_64_sync_handler+0x1c/0xf8
>  el0t_64_sync+0x180/0x188
>
> Adding NULL pointer checks to ensure stability by preventing operations
> on an invalid CRTC state.

Applied to mediatek-drm-fixes [1], thanks.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/chunkuang.hu/linux.git/log/?h=mediatek-drm-fixes

Regards,
Chun-Kuang.

>
> Fixes: d208261e9f7c ("drm/mediatek: Add wait_event_timeout when disabling plane")
> Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
> ---
> RESEND change:
> - Update author and Signed-off-by name.
> ---
>  drivers/gpu/drm/mediatek/mtk_plane.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_plane.c b/drivers/gpu/drm/mediatek/mtk_plane.c
> index cbc4f37da8ba..02349bd44001 100644
> --- a/drivers/gpu/drm/mediatek/mtk_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_plane.c
> @@ -292,7 +292,8 @@ static void mtk_plane_atomic_disable(struct drm_plane *plane,
>         wmb(); /* Make sure the above parameter is set before update */
>         mtk_plane_state->pending.dirty = true;
>
> -       mtk_crtc_plane_disable(old_state->crtc, plane);
> +       if (old_state && old_state->crtc)
> +               mtk_crtc_plane_disable(old_state->crtc, plane);
>  }
>
>  static void mtk_plane_atomic_update(struct drm_plane *plane,
> --
> 2.43.0
>

Re: [RESEND PATCH] drm/mediatek: Add error handling for old state CRTC in atomic_disable

Posted by CK Hu (胡俊光) 2 months, 1 week ago

On Mon, 2025-07-28 at 10:48 +0800, Jason-JH Lin wrote:
> Introduce error handling to address an issue where, after a hotplug
> event, the cursor continues to update. This situation can lead to a
> kernel panic due to accessing the NULL `old_state->crtc`.
> 
> E,g.
> Unable to handle kernel NULL pointer dereference at virtual address
> Call trace:
>  mtk_crtc_plane_disable+0x24/0x140
>  mtk_plane_atomic_update+0x8c/0xa8
>  drm_atomic_helper_commit_planes+0x114/0x2c8
>  drm_atomic_helper_commit_tail_rpm+0x4c/0x158
>  commit_tail+0xa0/0x168
>  drm_atomic_helper_commit+0x110/0x120
>  drm_atomic_commit+0x8c/0xe0
>  drm_atomic_helper_update_plane+0xd4/0x128
>  __setplane_atomic+0xcc/0x110
>  drm_mode_cursor_common+0x250/0x440
>  drm_mode_cursor_ioctl+0x44/0x70
>  drm_ioctl+0x264/0x5d8
>  __arm64_sys_ioctl+0xd8/0x510
>  invoke_syscall+0x6c/0xe0
>  do_el0_svc+0x68/0xe8
>  el0_svc+0x34/0x60
>  el0t_64_sync_handler+0x1c/0xf8
>  el0t_64_sync+0x180/0x188
> 
> Adding NULL pointer checks to ensure stability by preventing operations
> on an invalid CRTC state.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Fixes: d208261e9f7c ("drm/mediatek: Add wait_event_timeout when disabling plane")
> Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
> ---
> RESEND change:
> - Update author and Signed-off-by name.
> ---
>  drivers/gpu/drm/mediatek/mtk_plane.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_plane.c b/drivers/gpu/drm/mediatek/mtk_plane.c
> index cbc4f37da8ba..02349bd44001 100644
> --- a/drivers/gpu/drm/mediatek/mtk_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_plane.c
> @@ -292,7 +292,8 @@ static void mtk_plane_atomic_disable(struct drm_plane *plane,
>  	wmb(); /* Make sure the above parameter is set before update */
>  	mtk_plane_state->pending.dirty = true;
>  
> -	mtk_crtc_plane_disable(old_state->crtc, plane);
> +	if (old_state && old_state->crtc)
> +		mtk_crtc_plane_disable(old_state->crtc, plane);
>  }
>  
>  static void mtk_plane_atomic_update(struct drm_plane *plane,

Re: [RESEND PATCH] drm/mediatek: Add error handling for old state CRTC in atomic_disable

Posted by AngeloGioacchino Del Regno 2 months, 1 week ago

Il 28/07/25 04:48, Jason-JH Lin ha scritto:
> Introduce error handling to address an issue where, after a hotplug
> event, the cursor continues to update. This situation can lead to a
> kernel panic due to accessing the NULL `old_state->crtc`.
> 
> E,g.
> Unable to handle kernel NULL pointer dereference at virtual address
> Call trace:
>   mtk_crtc_plane_disable+0x24/0x140
>   mtk_plane_atomic_update+0x8c/0xa8
>   drm_atomic_helper_commit_planes+0x114/0x2c8
>   drm_atomic_helper_commit_tail_rpm+0x4c/0x158
>   commit_tail+0xa0/0x168
>   drm_atomic_helper_commit+0x110/0x120
>   drm_atomic_commit+0x8c/0xe0
>   drm_atomic_helper_update_plane+0xd4/0x128
>   __setplane_atomic+0xcc/0x110
>   drm_mode_cursor_common+0x250/0x440
>   drm_mode_cursor_ioctl+0x44/0x70
>   drm_ioctl+0x264/0x5d8
>   __arm64_sys_ioctl+0xd8/0x510
>   invoke_syscall+0x6c/0xe0
>   do_el0_svc+0x68/0xe8
>   el0_svc+0x34/0x60
>   el0t_64_sync_handler+0x1c/0xf8
>   el0t_64_sync+0x180/0x188
> 
> Adding NULL pointer checks to ensure stability by preventing operations
> on an invalid CRTC state.
> 
> Fixes: d208261e9f7c ("drm/mediatek: Add wait_event_timeout when disabling plane")
> Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>