Unverified Commit 7149be78 authored by Shenghao Yang's avatar Shenghao Yang Committed by Ruben Wauters
Browse files

drm/gud: fix NULL crtc dereference on display disable 

gud_plane_atomic_update() currently handles both crtc state and
framebuffer updates - the complexity has led to a few accidental
NULL pointer dereferences.

Commit dc2d5ddb ("drm/gud: fix NULL fb and crtc dereferences
on USB disconnect") [1] fixed an earlier dereference but planes
can also be disabled in non-hotplug paths (e.g. display disables
via the desktop environment). The drm_dev_enter() call would not
cause an early return in those and 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

Split out crtc handling from gud_plane_atomic_update() into
atomic_enable() and atomic_disable() functions to delegate
crtc state transitioning work to the DRM helpers.

To preserve the gud state commit sequence [2], switch to
the runtime PM version of drm_atomic_helper_commit_tail() which
ensures that crtcs are enabled (hence sending the
GUD_REQ_SET_CONTROLLER_ENABLE and GUD_REQ_SET_DISPLAY_ENABLE
requests) before a framebuffer update is sent.

[1] https://lore.kernel.org/all/20251231055039.44266-1-me@shenghaoyang.info/
[2] https://github.com/notro/gud/wiki/GUD-Protocol#display-state



Reported-by: default avatarkernel test robot <lkp@intel.com>
Reported-by: default avatarDan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/r/202601142159.0v8ilfVs-lkp@intel.com/


Fixes: 73cfd166 ("drm/gud: Replace simple display pipe with DRM atomic helpers")
Cc: <stable@vger.kernel.org> # 6.19.x
Cc: <stable@vger.kernel.org> # 6.18.x
Signed-off-by: default avatarShenghao Yang <me@shenghaoyang.info>
Reviewed-by: default avatarThomas Zimmermann <tzimmermann@suse.de>
Acked-by: default avatarRuben Wauters <rubenru09@aol.com>
Signed-off-by: default avatarRuben Wauters <rubenru09@aol.com>
Link: https://patch.msgid.link/20260222054551.80864-1-me@shenghaoyang.info
parent 46d8a07b

drivers/gpu/drm/gud/gud_drv.c

+8 −1
Original line number Diff line number Diff line
@@ -339,7 +339,9 @@ static int gud_stats_debugfs(struct seq_file *m, void *data) 
}



static const struct drm_crtc_helper_funcs gud_crtc_helper_funcs = {

	.atomic_check = drm_crtc_helper_atomic_check

	.atomic_check = drm_crtc_helper_atomic_check,

	.atomic_enable = gud_crtc_atomic_enable,

	.atomic_disable = gud_crtc_atomic_disable,

};



static const struct drm_crtc_funcs gud_crtc_funcs = {
@@ -364,6 +366,10 @@ static const struct drm_plane_funcs gud_plane_funcs = { 
	DRM_GEM_SHADOW_PLANE_FUNCS,

};



static const struct drm_mode_config_helper_funcs gud_mode_config_helpers = {

	.atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,

};



static const struct drm_mode_config_funcs gud_mode_config_funcs = {

	.fb_create = drm_gem_fb_create_with_dirty,

	.atomic_check = drm_atomic_helper_check,
@@ -499,6 +505,7 @@ static int gud_probe(struct usb_interface *intf, const struct usb_device_id *id) 
	drm->mode_config.min_height = le32_to_cpu(desc.min_height);

	drm->mode_config.max_height = le32_to_cpu(desc.max_height);

	drm->mode_config.funcs = &gud_mode_config_funcs;

	drm->mode_config.helper_private = &gud_mode_config_helpers;



	/* Format init */

	formats_dev = devm_kmalloc(dev, GUD_FORMATS_MAX_NUM, GFP_KERNEL);

drivers/gpu/drm/gud/gud_internal.h

+4 −0
Original line number Diff line number Diff line
@@ -62,6 +62,10 @@ int gud_usb_set_u8(struct gud_device *gdrm, u8 request, u8 val); 


void gud_clear_damage(struct gud_device *gdrm);

void gud_flush_work(struct work_struct *work);

void gud_crtc_atomic_enable(struct drm_crtc *crtc,

			    struct drm_atomic_state *state);

void gud_crtc_atomic_disable(struct drm_crtc *crtc,

			     struct drm_atomic_state *state);

int gud_plane_atomic_check(struct drm_plane *plane,

			   struct drm_atomic_state *state);

void gud_plane_atomic_update(struct drm_plane *plane,

drivers/gpu/drm/gud/gud_pipe.c

+36 −18
Original line number Diff line number Diff line
@@ -580,6 +580,39 @@ int gud_plane_atomic_check(struct drm_plane *plane, 
	return ret;

}



void gud_crtc_atomic_enable(struct drm_crtc *crtc,

			    struct drm_atomic_state *state)

{

	struct drm_device *drm = crtc->dev;

	struct gud_device *gdrm = to_gud_device(drm);

	int idx;



	if (!drm_dev_enter(drm, &idx))

		return;



	gud_usb_set_u8(gdrm, GUD_REQ_SET_CONTROLLER_ENABLE, 1);

	gud_usb_set(gdrm, GUD_REQ_SET_STATE_COMMIT, 0, NULL, 0);

	gud_usb_set_u8(gdrm, GUD_REQ_SET_DISPLAY_ENABLE, 1);



	drm_dev_exit(idx);

}



void gud_crtc_atomic_disable(struct drm_crtc *crtc,

			     struct drm_atomic_state *state)

{

	struct drm_device *drm = crtc->dev;

	struct gud_device *gdrm = to_gud_device(drm);

	int idx;



	if (!drm_dev_enter(drm, &idx))

		return;



	gud_usb_set_u8(gdrm, GUD_REQ_SET_DISPLAY_ENABLE, 0);

	gud_usb_set_u8(gdrm, GUD_REQ_SET_CONTROLLER_ENABLE, 0);



	drm_dev_exit(idx);

}



void gud_plane_atomic_update(struct drm_plane *plane,

			     struct drm_atomic_state *atomic_state)

{
@@ -607,24 +640,12 @@ void gud_plane_atomic_update(struct drm_plane *plane, 
		mutex_unlock(&gdrm->damage_lock);

	}



	if (!drm_dev_enter(drm, &idx))

	if (!crtc || !drm_dev_enter(drm, &idx))

		return;



	if (!old_state->fb)

		gud_usb_set_u8(gdrm, GUD_REQ_SET_CONTROLLER_ENABLE, 1);



	if (fb && (crtc->state->mode_changed || crtc->state->connectors_changed))

		gud_usb_set(gdrm, GUD_REQ_SET_STATE_COMMIT, 0, NULL, 0);



	if (crtc->state->active_changed)

		gud_usb_set_u8(gdrm, GUD_REQ_SET_DISPLAY_ENABLE, crtc->state->active);



	if (!fb)

		goto ctrl_disable;



	ret = drm_gem_fb_begin_cpu_access(fb, DMA_FROM_DEVICE);

	if (ret)

		goto ctrl_disable;

		goto out;



	drm_atomic_helper_damage_iter_init(&iter, old_state, new_state);

	drm_atomic_for_each_plane_damage(&iter, &damage)
@@ -632,9 +653,6 @@ 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)

		gud_usb_set_u8(gdrm, GUD_REQ_SET_CONTROLLER_ENABLE, 0);



out:

	drm_dev_exit(idx);

}