1. Patchew
  2. linux
  3. View series

[PATCH] drm/gud: fix NULL crtc dereference on display disable

Shenghao Yang posted 1 patch 3 weeks ago
There is a newer version of this series
drivers/gpu/drm/gud/gud_pipe.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[PATCH] drm/gud: fix NULL crtc dereference on display disable
Posted by Shenghao Yang 3 weeks ago 
Commit dc2d5ddb193e ("drm/gud: fix NULL fb and crtc dereferences
on USB disconnect") [1] only fixed the initial NULL crtc dereference
in gud_plane_atomic_update().

However, planes can also be disabled in non-hotplug paths (e.g.
display disables via the DE). The drm_dev_enter() call would not
cause an early return in those and we'll subsequently oops on
dereferencing crtc:

BUG: kernel NULL pointer dereference, address: 00000000000005c8
CPU: 6 UID: 1000 PID: 3473 Comm: kwin_wayland Not tainted 6.18.2-200.vanilla.gud.fc42.x86_64 #1 PREEMPT(lazy)
RIP: 0010:gud_plane_atomic_update+0x148/0x470 [gud]
 <TASK>
 drm_atomic_helper_commit_planes+0x28e/0x310
 drm_atomic_helper_commit_tail+0x2a/0x70
 commit_tail+0xf1/0x150
 drm_atomic_helper_commit+0x13c/0x180
 drm_atomic_commit+0xb1/0xe0
info ? __pfx___drm_printfn_info+0x10/0x10
 drm_mode_atomic_ioctl+0x70f/0x7c0
 ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
 drm_ioctl_kernel+0xae/0x100
 drm_ioctl+0x2a8/0x550
 ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
 __x64_sys_ioctl+0x97/0xe0
 do_syscall_64+0x7e/0x7f0
 ? __ct_user_enter+0x56/0xd0
 ? do_syscall_64+0x158/0x7f0
 ? __ct_user_enter+0x56/0xd0
 ? do_syscall_64+0x158/0x7f0
 entry_SYSCALL_64_after_hwframe+0x76/0x7e

Add an early exit and disable the display controller if crtc is NULL.

[1] https://lore.kernel.org/all/20251231055039.44266-1-me@shenghaoyang.info/

Cc: <stable@vger.kernel.org> # 6.18.x
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/202601142159.0v8ilfVs-lkp@intel.com/
Fixes: 73cfd166e045 ("drm/gud: Replace simple display pipe with DRM atomic helpers")
Signed-off-by: Shenghao Yang <me@shenghaoyang.info>
---
 drivers/gpu/drm/gud/gud_pipe.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
index 4b77be94348d..a69dee71490f 100644
--- a/drivers/gpu/drm/gud/gud_pipe.c
+++ b/drivers/gpu/drm/gud/gud_pipe.c
@@ -610,6 +610,9 @@ void gud_plane_atomic_update(struct drm_plane *plane,
 	if (!drm_dev_enter(drm, &idx))
 		return;
 
+	if (!crtc)
+		goto ctrl_disable;
+
 	if (!old_state->fb)
 		gud_usb_set_u8(gdrm, GUD_REQ_SET_CONTROLLER_ENABLE, 1);
 
@@ -633,7 +636,7 @@ void gud_plane_atomic_update(struct drm_plane *plane,
 	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
 
 ctrl_disable:
-	if (!crtc->state->enable)
+	if (!crtc || !crtc->state->enable)
 		gud_usb_set_u8(gdrm, GUD_REQ_SET_CONTROLLER_ENABLE, 0);
 
 	drm_dev_exit(idx);
-- 
2.52.0
Re: [PATCH] drm/gud: fix NULL crtc dereference on display disable
Posted by Thomas Zimmermann 2 weeks, 6 days ago 
Hi,

thanks for the patch.


Am 18.01.26 um 13:50 schrieb Shenghao Yang:
> Commit dc2d5ddb193e ("drm/gud: fix NULL fb and crtc dereferences
> on USB disconnect") [1] only fixed the initial NULL crtc dereference
> in gud_plane_atomic_update().
>
> However, planes can also be disabled in non-hotplug paths (e.g.
> display disables via the DE). The drm_dev_enter() call would not

'DE' ?

> cause an early return in those and we'll subsequently oops on
> dereferencing crtc:
>
> BUG: kernel NULL pointer dereference, address: 00000000000005c8
> CPU: 6 UID: 1000 PID: 3473 Comm: kwin_wayland Not tainted 6.18.2-200.vanilla.gud.fc42.x86_64 #1 PREEMPT(lazy)
> RIP: 0010:gud_plane_atomic_update+0x148/0x470 [gud]
>   <TASK>
>   drm_atomic_helper_commit_planes+0x28e/0x310
>   drm_atomic_helper_commit_tail+0x2a/0x70
>   commit_tail+0xf1/0x150
>   drm_atomic_helper_commit+0x13c/0x180
>   drm_atomic_commit+0xb1/0xe0
> info ? __pfx___drm_printfn_info+0x10/0x10
>   drm_mode_atomic_ioctl+0x70f/0x7c0
>   ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
>   drm_ioctl_kernel+0xae/0x100
>   drm_ioctl+0x2a8/0x550
>   ? __pfx_drm_mode_atomic_ioctl+0x10/0x10
>   __x64_sys_ioctl+0x97/0xe0
>   do_syscall_64+0x7e/0x7f0
>   ? __ct_user_enter+0x56/0xd0
>   ? do_syscall_64+0x158/0x7f0
>   ? __ct_user_enter+0x56/0xd0
>   ? do_syscall_64+0x158/0x7f0
>   entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
> Add an early exit and disable the display controller if crtc is NULL.

It seems to me that all these calls to GUD_REQ_SET_CONTROLLER_ENABLE(^1) 
and GUD_REQ_SET_DISPLAY_ENABLEshould rather go to the CRTC's 
atomic_enable/atomic_disable functions. Those currently seem missing 
from [1]. The atomic_update helper would then be reduced to damage 
handling. Best regards Thomas [1] 
https://elixir.bootlin.com/linux/v6.18.6/source/drivers/gpu/drm/gud/gud_drv.c#L341 

^1: SET_CONTROLLER_ENABLE sounds like it could even be part of device 
probing and runtime PM management, but that is a more invasive change.
>
> [1] https://lore.kernel.org/all/20251231055039.44266-1-me@shenghaoyang.info/
>
> Cc: <stable@vger.kernel.org> # 6.18.x
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/r/202601142159.0v8ilfVs-lkp@intel.com/
> Fixes: 73cfd166e045 ("drm/gud: Replace simple display pipe with DRM atomic helpers")
> Signed-off-by: Shenghao Yang <me@shenghaoyang.info>
> ---
>   drivers/gpu/drm/gud/gud_pipe.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/gud/gud_pipe.c b/drivers/gpu/drm/gud/gud_pipe.c
> index 4b77be94348d..a69dee71490f 100644
> --- a/drivers/gpu/drm/gud/gud_pipe.c
> +++ b/drivers/gpu/drm/gud/gud_pipe.c
> @@ -610,6 +610,9 @@ void gud_plane_atomic_update(struct drm_plane *plane,
>   	if (!drm_dev_enter(drm, &idx))
>   		return;
>   
> +	if (!crtc)
> +		goto ctrl_disable;
> +
>   	if (!old_state->fb)
>   		gud_usb_set_u8(gdrm, GUD_REQ_SET_CONTROLLER_ENABLE, 1);
>   
> @@ -633,7 +636,7 @@ void gud_plane_atomic_update(struct drm_plane *plane,
>   	drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE);
>   
>   ctrl_disable:
> -	if (!crtc->state->enable)
> +	if (!crtc || !crtc->state->enable)
>   		gud_usb_set_u8(gdrm, GUD_REQ_SET_CONTROLLER_ENABLE, 0);
>   
>   	drm_dev_exit(idx);

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
Re: [PATCH] drm/gud: fix NULL crtc dereference on display disable
Posted by Shenghao Yang 2 weeks, 6 days ago 
Hi,

On 19/1/26 16:17, Thomas Zimmermann wrote:
> Hi,
> 
> thanks for the patch.

Thanks for taking a look. I did forget about this oops and the
smatch failure was a good reminder!

> 
> Am 18.01.26 um 13:50 schrieb Shenghao Yang:
>> Commit dc2d5ddb193e ("drm/gud: fix NULL fb and crtc dereferences
>> on USB disconnect") [1] only fixed the initial NULL crtc dereference
>> in gud_plane_atomic_update().
>>
>> However, planes can also be disabled in non-hotplug paths (e.g.
>> display disables via the DE). The drm_dev_enter() call would not
> 
> 'DE' ?

Ah - the desktop environment. I was scratching my head for why the
box kept oops-ing on boot even after the hotplug fix. It turned out
kscreen was applying the saved "disable display" setting.

> It seems to me that all these calls to GUD_REQ_SET_CONTROLLER_ENABLE(^1) and GUD_REQ_SET_DISPLAY_ENABLEshould rather go to the CRTC's atomic_enable/atomic_disable functions. Those currently seem missing from [1]. The atomic_update helper would then be reduced to damage handling. Best regards Thomas [1] https://elixir.bootlin.com/linux/v6.18.6/source/drivers/gpu/drm/gud/gud_drv.c#L341
> ^1: SET_CONTROLLER_ENABLE sounds like it could even be part of device probing and runtime PM management, but that is a more invasive change.

That feels like it'd be much cleaner. I'll respin with that in v2.

Shenghao

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.