[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <fa36159a-fa41-4066-abea-60a439e944b3@suse.de>
Date: Mon, 19 Jan 2026 09:17:31 +0100
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Shenghao Yang <me@...nghaoyang.info>, Ruben Wauters <rubenru09@....com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
stable@...r.kernel.org, kernel test robot <lkp@...el.com>,
Dan Carpenter <dan.carpenter@...aro.org>
Subject: Re: [PATCH] drm/gud: fix NULL crtc dereference on display disable
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@...r.kernel.org> # 6.18.x
> Reported-by: kernel test robot <lkp@...el.com>
> Reported-by: Dan Carpenter <dan.carpenter@...aro.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@...nghaoyang.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)
Powered by blists - more mailing lists