FAILED: patch "[PATCH] drm/client: fix restore of partially initialized client" failed to apply to 6.18-stable tree
Subsystems (3)
[email protected]
maintainerThe patch below does not apply to the 6.18-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.18.y
git checkout FETCH_HEAD
git cherry-pick -x 1fca688e9443003e33cf30453e7a7560367656c9
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026092902-autistic-bonnet-dcb5@gregkh' --subject-prefix 'PATCH 6.18.y' 'HEAD^..'
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 1fca688e9443003e33cf30453e7a7560367656c9 Mon Sep 17 00:00:00 2001
From: shechenglong <[email protected]>
Date: Mon, 7 Sep 2026 11:51:47 +0800
Subject: [PATCH] drm/client: fix restore of partially initialized client
I got a null-ptr-deref report when closing a DRM file descriptor:
WARNING: drivers/gpu/drm/drm_atomic.c:2031 at __drm_atomic_helper_set_config+0x18e/0x1b0 [drm] Call Trace: drm_client_modeset_commit_atomic+0x16b/0x220 [drm] drm_client_modeset_commit_locked+0x56/0x160 [drm] drm_client_modeset_commit+0x21/0x40 [drm] __drm_fb_helper_restore_fbdev_mode_unlocked.part.0+0x7b/0x80 drm_fbdev_client_restore+0xe/0x20 [drm_client_lib] drm_client_dev_restore+0x9f/0xc0 [drm] drm_release+0xc5/0xe0 [drm]
The warning is followed by a NULL pointer dereference:
BUG: kernel NULL pointer dereference, address: 0000000000000008
RIP:
__drm_fb_helper_restore_fbdev_mode_unlocked.part.0+0x41/0x80 [drm_kms_helper]
Call Trace: drm_fbdev_client_restore+0xe/0x20 [drm_client_lib] drm_client_dev_restore+0x9f/0xc0 [drm] drm_release+0xc5/0xe0 [drm] __fput+0xdc/0x2b0 __x64_sys_close+0x39/0x80 do_syscall_64+0x8d/0x460 entry_SYSCALL_64_after_hwframe+0x76/0x7e
drm_client_register() adds the DRM client to the device client list before invoking the initial hotplug callback. If the hotplug callback fails, the client remains registered.
For the fbdev client, a failure during drm_fb_helper_initial_config() causes the partially initialized fbdev helper to be cleaned up. drm_fb_helper_fini() releases fb_helper->info and leaves it NULL.
The fbdev client therefore remains registered even though there is no fully initialized framebuffer device.
Later, when userspace closes the DRM file descriptor, drm_release() can invoke the restore callbacks of registered DRM clients:
drm_release()drm_client_dev_restore()drm_fbdev_client_restore()drm_fb_helper_restore_fbdev_mode_unlocked()
drm_fbdev_client_restore() currently restores the fbdev state unconditionally. For a partially initialized fbdev client this can submit an incomplete modeset state and subsequently access fbdev state which has not been initialized, resulting in the warning and NULL pointer dereference above.
drm_fbdev_client_unregister() already uses fb_helper->info to distinguish a fully probed framebuffer device from a partially initialized client.
Use the same condition in drm_fbdev_client_restore() and skip restore if no framebuffer device has been successfully initialized.
Signed-off-by shechenglong <[email protected]>
Reviewed-by Thomas Zimmermann <[email protected]>
Fixes 5d08c44e47b9 ("drm/fbdev: Add memory-agnostic fbdev client")
Signed-off-by Thomas Zimmermann <[email protected]>
Cc <[email protected]> # v6.13+
drivers/gpu/drm/clients/drm_fbdev_client.c +8 -0
@@ -42,6 +42,14 @@ static int drm_fbdev_client_restore(struct drm_client_dev *client, bool force) {struct drm_fb_helper *fb_helper =drm_fb_helper_from_client (client); +/* +* The client is registered before the initial fbdev probe. +* If probing failed, the client remains registered but there +* is no valid fbdev framebuffer to restore. +*/ +if (!fb_helper->info || !fb_helper->fb) +return 0 ; +drm_fb_helper_restore_fbdev_mode_unlocked (fb_helper, force);return 0 ;
Sasha Levin
maintainerPATCH 1/3 drm/client: Remove holds_console_lock parameter from suspend/resume
From: Thomas Zimmermann <[email protected]>
[ Upstream commit 7910d69376cde30e5871970d97d1a2e360568474 ]
No caller of the client resume/suspend helpers holds the console lock. The last such cases were removed from radeon in the patch series at [1]. Now remove the related parameter and the TODO items.
v2:
- update placeholders for CONFIG_DRM_CLIENT=n
Signed-off-by Thomas Zimmermann <[email protected]>
Reviewed-by Rodrigo Vivi <[email protected]>
Acked-by Rodrigo Vivi <[email protected]>
Reviewed-by Petr Vorel <[email protected]>
Reviewed-by Andi Shyti <[email protected]>
Acked-by Danilo Krummrich <[email protected]>
Reviewed-by Lyude Paul <[email protected]>
Reviewed-by Jocelyn Falempe <[email protected]>
Stable-dep-of: 1fca688e9443 ("drm/client: fix restore of partially initialized client")
Signed-off-by Sasha Levin <[email protected]>
--- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++++---- drivers/gpu/drm/clients/drm_fbdev_client.c | 14 ++++---------- drivers/gpu/drm/clients/drm_log.c | 4 ++-- drivers/gpu/drm/drm_client_event.c | 16 ++++++++-------- drivers/gpu/drm/drm_modeset_helper.c | 6 +++--- drivers/gpu/drm/i915/i915_driver.c | 6 +++--- drivers/gpu/drm/nouveau/nouveau_display.c | 4 ++-- drivers/gpu/drm/radeon/radeon_device.c | 4 ++-- drivers/gpu/drm/xe/display/xe_display.c | 6 +++--- include/drm/drm_client.h | 14 ++------------ include/drm/drm_client_event.h | 8 ++++---- 11 files changed, 37 insertions(+), 53 deletions(-)
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +4 -4
@@ -5315,7 +5315,7 @@ int amdgpu_device_suspend(struct drm_device *dev, bool notify_clients)dev_warn (adev->dev,"smart shift update failed\n" );if (notify_clients) -drm_client_dev_suspend (adev_to_drm (adev), false); +drm_client_dev_suspend (adev_to_drm (adev));cancel_delayed_work_sync (&adev->delayed_init_work);
@@ -5445,7 +5445,7 @@ int amdgpu_device_resume(struct drm_device *dev, bool notify_clients)flush_delayed_work (&adev->delayed_init_work);if (notify_clients) -drm_client_dev_resume (adev_to_drm (adev), false); +drm_client_dev_resume (adev_to_drm (adev));amdgpu_ras_resume (adev);
@@ -6060,7 +6060,7 @@ int amdgpu_device_reinit_after_reset(struct amdgpu_reset_context *reset_context)if (r) goto out; -drm_client_dev_resume (adev_to_drm (tmp_adev), false); +drm_client_dev_resume (adev_to_drm (tmp_adev));/* * The GPU enters bad state once faulty pages
@@ -6395,7 +6395,7 @@ static void amdgpu_device_halt_activities(struct amdgpu_device *adev,*/ amdgpu_unregister_gpu_instance (tmp_adev); -drm_client_dev_suspend (adev_to_drm (tmp_adev), false); +drm_client_dev_suspend (adev_to_drm (tmp_adev));/* disable ras on ALL IPs */ if (!need_emergency_restart && !amdgpu_reset_in_dpc(adev) &&
drivers/gpu/drm/clients/drm_fbdev_client.c +4 -10
@@ -62,26 +62,20 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)return ret; } -static int drm_fbdev_client_suspend (struct drm_client_dev *client,bool holds_console_lock) +static int drm_fbdev_client_suspend (struct drm_client_dev *client) {struct drm_fb_helper *fb_helper =drm_fb_helper_from_client (client); -if (holds_console_lock) -drm_fb_helper_set_suspend (fb_helper, true); -else -drm_fb_helper_set_suspend_unlocked (fb_helper, true); +drm_fb_helper_set_suspend_unlocked (fb_helper, true);return 0 ; } -static int drm_fbdev_client_resume (struct drm_client_dev *client,bool holds_console_lock) +static int drm_fbdev_client_resume (struct drm_client_dev *client) {struct drm_fb_helper *fb_helper =drm_fb_helper_from_client (client); -if (holds_console_lock) -drm_fb_helper_set_suspend (fb_helper, false); -else -drm_fb_helper_set_suspend_unlocked (fb_helper, false); +drm_fb_helper_set_suspend_unlocked (fb_helper, false);return 0 ; }
drivers/gpu/drm/clients/drm_log.c +2 -2
@@ -328,7 +328,7 @@ static int drm_log_client_hotplug(struct drm_client_dev *client)return 0 ; } -static int drm_log_client_suspend (struct drm_client_dev *client,bool _console_lock) +static int drm_log_client_suspend (struct drm_client_dev *client) {struct drm_log *dlog =client_to_drm_log (client);
@@ -337,7 +337,7 @@ static int drm_log_client_suspend(struct drm_client_dev *client, bool _console_lreturn 0 ; } -static int drm_log_client_resume (struct drm_client_dev *client,bool _console_lock) +static int drm_log_client_resume (struct drm_client_dev *client) {struct drm_log *dlog =client_to_drm_log (client);
drivers/gpu/drm/drm_client_event.c +8 -8
@@ -122,7 +122,7 @@ void drm_client_dev_restore(struct drm_device *dev)mutex_unlock (&dev->clientlist_mutex); } -static int drm_client_suspend (struct drm_client_dev *client,bool holds_console_lock) +static int drm_client_suspend (struct drm_client_dev *client) {struct drm_device *dev = client->dev;int ret =0 ;
@@ -131,7 +131,7 @@ static int drm_client_suspend(struct drm_client_dev *client, bool holds_console_return 0 ;if (client->funcs && client->funcs->suspend) - ret = client->funcs->suspend (client, holds_console_lock); + ret = client->funcs->suspend (client);drm_dbg_kms (dev,"%s: ret=%d\n" , client->name, ret); client->suspended = true;
@@ -139,20 +139,20 @@ static int drm_client_suspend(struct drm_client_dev *client, bool holds_console_return ret; } -void drm_client_dev_suspend (struct drm_device *dev,bool holds_console_lock) +void drm_client_dev_suspend (struct drm_device *dev) {struct drm_client_dev *client;mutex_lock (&dev->clientlist_mutex);list_for_each_entry (client, &dev->clientlist, list) {if (!client->suspended) -drm_client_suspend (client, holds_console_lock); +drm_client_suspend (client); }mutex_unlock (&dev->clientlist_mutex); }EXPORT_SYMBOL (drm_client_dev_suspend); -static int drm_client_resume (struct drm_client_dev *client,bool holds_console_lock) +static int drm_client_resume (struct drm_client_dev *client) {struct drm_device *dev = client->dev;int ret =0 ;
@@ -161,7 +161,7 @@ static int drm_client_resume(struct drm_client_dev *client, bool holds_console_lreturn 0 ;if (client->funcs && client->funcs->resume) - ret = client->funcs->resume (client, holds_console_lock); + ret = client->funcs->resume (client);drm_dbg_kms (dev,"%s: ret=%d\n" , client->name, ret); client->suspended = false;
@@ -172,14 +172,14 @@ static int drm_client_resume(struct drm_client_dev *client, bool holds_console_lreturn ret; } -void drm_client_dev_resume (struct drm_device *dev,bool holds_console_lock) +void drm_client_dev_resume (struct drm_device *dev) {struct drm_client_dev *client;mutex_lock (&dev->clientlist_mutex);list_for_each_entry (client, &dev->clientlist, list) {if (client->suspended) -drm_client_resume (client, holds_console_lock); +drm_client_resume (client); }mutex_unlock (&dev->clientlist_mutex); }
drivers/gpu/drm/drm_modeset_helper.c +3 -3
@@ -203,10 +203,10 @@ int drm_mode_config_helper_suspend(struct drm_device *dev)if (dev->mode_config.poll_enabled)drm_kms_helper_poll_disable (dev); -drm_client_dev_suspend (dev, false); +drm_client_dev_suspend (dev); state =drm_atomic_helper_suspend (dev);if (IS_ERR (state)) { -drm_client_dev_resume (dev, false); +drm_client_dev_resume (dev);/* * Don't enable polling if it was never initialized
@@ -252,7 +252,7 @@ int drm_mode_config_helper_resume(struct drm_device *dev)DRM_ERROR ("Failed to resume (%d)\n" , ret); dev->mode_config.suspend_state =NULL ; -drm_client_dev_resume (dev, false); +drm_client_dev_resume (dev);/* * Don't enable polling if it is not initialized
drivers/gpu/drm/i915/i915_driver.c +3 -3
@@ -978,7 +978,7 @@ void i915_driver_shutdown(struct drm_i915_private *i915)intel_runtime_pm_disable (&i915->runtime_pm);intel_power_domains_disable (display); -drm_client_dev_suspend (&i915->drm, false); +drm_client_dev_suspend (&i915->drm);if (intel_display_device_present (display)) {drm_kms_helper_poll_disable (&i915->drm);intel_display_driver_disable_user_access (display);
@@ -1061,7 +1061,7 @@ static int i915_drm_suspend(struct drm_device *dev)/* We do a lot of poking in a lot of registers, make sure they work * properly. */ intel_power_domains_disable (display); -drm_client_dev_suspend (dev, false); +drm_client_dev_suspend (dev);if (intel_display_device_present (display)) {drm_kms_helper_poll_disable (dev);intel_display_driver_disable_user_access (display);
@@ -1245,7 +1245,7 @@ static int i915_drm_resume(struct drm_device *dev)intel_opregion_resume (display); -drm_client_dev_resume (dev, false); +drm_client_dev_resume (dev);intel_power_domains_enable (display);
drivers/gpu/drm/nouveau/nouveau_display.c +2 -2
@@ -764,7 +764,7 @@ nouveau_display_suspend(struct drm_device *dev, bool runtime) {struct nouveau_display *disp =nouveau_display (dev); -drm_client_dev_suspend (dev, false); +drm_client_dev_suspend (dev);if (drm_drv_uses_atomic_modeset (dev)) {if (!runtime) {
@@ -795,7 +795,7 @@ nouveau_display_resume(struct drm_device *dev, bool runtime) } } -drm_client_dev_resume (dev, false); +drm_client_dev_resume (dev); }int
drivers/gpu/drm/radeon/radeon_device.c +2 -2
@@ -1635,7 +1635,7 @@ int radeon_suspend_kms(struct drm_device *dev, bool suspend, }if (notify_clients) -drm_client_dev_suspend (dev, false); +drm_client_dev_suspend (dev);return 0 ; }
@@ -1739,7 +1739,7 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool notify_clients)radeon_pm_compute_clocks (rdev);if (notify_clients) -drm_client_dev_resume (dev, false); +drm_client_dev_resume (dev);return 0 ; }
drivers/gpu/drm/xe/display/xe_display.c +3 -3
@@ -331,7 +331,7 @@ void xe_display_pm_suspend(struct xe_device *xe)* properly. */ intel_power_domains_disable (display); -drm_client_dev_suspend (&xe->drm, false); +drm_client_dev_suspend (&xe->drm);if (intel_display_device_present (display)) {drm_kms_helper_poll_disable (&xe->drm);
@@ -363,7 +363,7 @@ void xe_display_pm_shutdown(struct xe_device *xe)return ;intel_power_domains_disable (display); -drm_client_dev_suspend (&xe->drm, false); +drm_client_dev_suspend (&xe->drm);if (intel_display_device_present (display)) {drm_kms_helper_poll_disable (&xe->drm);
@@ -488,7 +488,7 @@ void xe_display_pm_resume(struct xe_device *xe)intel_opregion_resume (display); -drm_client_dev_resume (&xe->drm, false); +drm_client_dev_resume (&xe->drm);intel_power_domains_enable (display); }
include/drm/drm_client.h +2 -12
@@ -70,13 +70,8 @@ struct drm_client_funcs {* Called when suspending the device. * * This callback is optional. -* -* FIXME: Some callers hold the console lock when invoking this -* function. This interferes with fbdev emulation, which -* also tries to acquire the lock. Push the console lock -* into the callback and remove 'holds_console_lock'. */ -int (*suspend)(struct drm_client_dev *client,bool holds_console_lock); +int (*suspend)(struct drm_client_dev *client);/** * @resume:
@@ -84,13 +79,8 @@ struct drm_client_funcs {* Called when resuming the device from suspend. * * This callback is optional. -* -* FIXME: Some callers hold the console lock when invoking this -* function. This interferes with fbdev emulation, which -* also tries to acquire the lock. Push the console lock -* into the callback and remove 'holds_console_lock'. */ -int (*resume)(struct drm_client_dev *client,bool holds_console_lock); +int (*resume)(struct drm_client_dev *client); };/**
include/drm/drm_client_event.h +4 -4
@@ -11,8 +11,8 @@ struct drm_device;void drm_client_dev_unregister (struct drm_device *dev);void drm_client_dev_hotplug (struct drm_device *dev);void drm_client_dev_restore (struct drm_device *dev); -void drm_client_dev_suspend (struct drm_device *dev,bool holds_console_lock); -void drm_client_dev_resume (struct drm_device *dev,bool holds_console_lock); +void drm_client_dev_suspend (struct drm_device *dev); +void drm_client_dev_resume (struct drm_device *dev);#else static inline void drm_client_dev_unregister (struct drm_device *dev) { }
@@ -20,9 +20,9 @@ static inline void drm_client_dev_hotplug(struct drm_device *dev) { }static inline void drm_client_dev_restore (struct drm_device *dev) { } -static inline void drm_client_dev_suspend (struct drm_device *dev,bool holds_console_lock) +static inline void drm_client_dev_suspend (struct drm_device *dev) { } -static inline void drm_client_dev_resume (struct drm_device *dev,bool holds_console_lock) +static inline void drm_client_dev_resume (struct drm_device *dev) { }#endif
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 2/3 drm/client: Pass force parameter to client restore
From: Thomas Zimmermann <[email protected]>
[ Upstream commit 943240d342f148896733eb6c7b223a08aa1f520a ]
Add force parameter to client restore and pass value through the layers. The only currently used value is false.
If force is true, the client should restore its display even if it does not hold the DRM master lock. This is be required for emergency output, such as sysrq.
While at it, inline drm_fb_helper_lastclose(), which is a trivial wrapper around drm_fb_helper_restore_fbdev_mode_unlocked().
Signed-off-by Thomas Zimmermann <[email protected]>
Reviewed-by Jocelyn Falempe <[email protected]>
Stable-dep-of: 1fca688e9443 ("drm/client: fix restore of partially initialized client")
Signed-off-by Sasha Levin <[email protected]>
--- drivers/gpu/drm/clients/drm_fbdev_client.c | 6 ++++-- drivers/gpu/drm/drm_client_event.c | 4 ++-- drivers/gpu/drm/drm_fb_helper.c | 24 ++++++---------------- drivers/gpu/drm/drm_file.c | 2 +- include/drm/drm_client.h | 8 +++++--- include/drm/drm_client_event.h | 4 ++-- include/drm/drm_fb_helper.h | 8 ++------ 7 files changed, 22 insertions(+), 34 deletions(-)
drivers/gpu/drm/clients/drm_fbdev_client.c +4 -2
@@ -26,9 +26,11 @@ static void drm_fbdev_client_unregister(struct drm_client_dev *client) } } -static int drm_fbdev_client_restore (struct drm_client_dev *client) +static int drm_fbdev_client_restore (struct drm_client_dev *client,bool force) { -drm_fb_helper_lastclose (client->dev); +struct drm_fb_helper *fb_helper =drm_fb_helper_from_client (client); + +drm_fb_helper_restore_fbdev_mode_unlocked (fb_helper, force);return 0 ; }
drivers/gpu/drm/drm_client_event.c +2 -2
@@ -101,7 +101,7 @@ void drm_client_dev_hotplug(struct drm_device *dev) }EXPORT_SYMBOL (drm_client_dev_hotplug); -void drm_client_dev_restore (struct drm_device *dev) +void drm_client_dev_restore (struct drm_device *dev,bool force) {struct drm_client_dev *client;int ret;
@@ -114,7 +114,7 @@ void drm_client_dev_restore(struct drm_device *dev)if (!client->funcs || !client->funcs->restore)continue ; - ret = client->funcs->restore (client); + ret = client->funcs->restore (client, force);drm_dbg_kms (dev,"%s: ret=%d\n" , client->name, ret);if (!ret)/* The first one to return zero gets the privilege to restore */ break ;
drivers/gpu/drm/drm_fb_helper.c +6 -18
@@ -253,6 +253,7 @@ __drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,/** * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration * @fb_helper: driver-allocated fbdev helper, can be NULL +* @force: ignore present DRM master * * This helper should be called from fbdev emulation's &drm_client_funcs.restore * callback. It ensures that the user isn't greeted with a black screen when the
@@ -261,9 +262,9 @@ __drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper,* Returns: * 0 on success, or a negative errno code otherwise. */ -int drm_fb_helper_restore_fbdev_mode_unlocked (struct drm_fb_helper *fb_helper) +int drm_fb_helper_restore_fbdev_mode_unlocked (struct drm_fb_helper *fb_helper,bool force) { -return __drm_fb_helper_restore_fbdev_mode_unlocked (fb_helper, false); +return __drm_fb_helper_restore_fbdev_mode_unlocked (fb_helper, force); }EXPORT_SYMBOL (drm_fb_helper_restore_fbdev_mode_unlocked);
@@ -1316,9 +1317,9 @@ int drm_fb_helper_set_par(struct fb_info *info)* the KDSET IOCTL with KD_TEXT, and only after that drops the master * status when exiting. * -* In the past this was caught by drm_fb_helper_lastclose(), but on -* modern systems where logind always keeps a drm fd open to orchestrate -* the vt switching, this doesn't work. +* In the past this was caught by drm_fb_helper_restore_fbdev_mode_unlocked(), +* but on modern systems where logind always keeps a drm fd open to +* orchestrate the vt switching, this doesn't work. * * To not break the userspace ABI we have this special case here, which * is only used for the above case. Everything else uses the normal
@@ -1946,16 +1947,3 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper)return 0 ; }EXPORT_SYMBOL (drm_fb_helper_hotplug_event); - -/** -* drm_fb_helper_lastclose - DRM driver lastclose helper for fbdev emulation -* @dev: DRM device -* -* This function is obsolete. Call drm_fb_helper_restore_fbdev_mode_unlocked() -* instead. -*/ -void drm_fb_helper_lastclose (struct drm_device *dev) -{ -drm_fb_helper_restore_fbdev_mode_unlocked (dev->fb_helper); -} -EXPORT_SYMBOL (drm_fb_helper_lastclose);
drivers/gpu/drm/drm_file.c +1 -1
@@ -405,7 +405,7 @@ EXPORT_SYMBOL(drm_open);static void drm_lastclose (struct drm_device *dev) { -drm_client_dev_restore (dev); +drm_client_dev_restore (dev, false);if (dev_is_pci (dev->dev))vga_switcheroo_process_delayed_switch ();
include/drm/drm_client.h +5 -3
@@ -47,12 +47,14 @@ struct drm_client_funcs {* * Note that the core does not guarantee exclusion against concurrent * drm_open(). Clients need to ensure this themselves, for example by -* using drm_master_internal_acquire() and -* drm_master_internal_release(). +* using drm_master_internal_acquire() and drm_master_internal_release(). +* +* If the caller passes force, the client should ignore any present DRM +* master and restore the display anyway. * * This callback is optional. */ -int (*restore)(struct drm_client_dev *client); +int (*restore)(struct drm_client_dev *client,bool force);/** * @hotplug:
include/drm/drm_client_event.h +2 -2
@@ -10,7 +10,7 @@ struct drm_device;#if defined(CONFIG_DRM_CLIENT )void drm_client_dev_unregister (struct drm_device *dev);void drm_client_dev_hotplug (struct drm_device *dev); -void drm_client_dev_restore (struct drm_device *dev); +void drm_client_dev_restore (struct drm_device *dev,bool force);void drm_client_dev_suspend (struct drm_device *dev);void drm_client_dev_resume (struct drm_device *dev);#else
@@ -18,7 +18,7 @@ static inline void drm_client_dev_unregister(struct drm_device *dev) { }static inline void drm_client_dev_hotplug (struct drm_device *dev) { } -static inline void drm_client_dev_restore (struct drm_device *dev) +static inline void drm_client_dev_restore (struct drm_device *dev,bool force) { }static inline void drm_client_dev_suspend (struct drm_device *dev) { }
include/drm/drm_fb_helper.h +2 -6
@@ -254,7 +254,8 @@ int drm_fb_helper_set_par(struct fb_info *info);int drm_fb_helper_check_var (struct fb_var_screeninfo *var,struct fb_info *info); -int drm_fb_helper_restore_fbdev_mode_unlocked (struct drm_fb_helper *fb_helper); +int drm_fb_helper_restore_fbdev_mode_unlocked (struct drm_fb_helper *fb_helper, +bool force);void drm_fb_helper_unregister_info (struct drm_fb_helper *fb_helper); void drm_fb_helper_fill_info(struct fb_info *info,
@@ -281,7 +282,6 @@ int drm_fb_helper_hotplug_event(struct drm_fb_helper *fb_helper);int drm_fb_helper_initial_config (struct drm_fb_helper *fb_helper);int drm_fb_helper_debug_enter (struct fb_info *info);int drm_fb_helper_debug_leave (struct fb_info *info); -void drm_fb_helper_lastclose (struct drm_device *dev);#else static inline void drm_fb_helper_prepare(struct drm_device *dev, struct drm_fb_helper *helper,
@@ -397,10 +397,6 @@ static inline int drm_fb_helper_debug_leave(struct fb_info *info) {return 0 ; } - -static inline void drm_fb_helper_lastclose (struct drm_device *dev) -{ -}#endif #endif
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 3/3 drm/client: fix restore of partially initialized client
In reply to Sasha LevinFrom: shechenglong <[email protected]>
[ Upstream commit 1fca688e9443003e33cf30453e7a7560367656c9 ]
I got a null-ptr-deref report when closing a DRM file descriptor:
WARNING: drivers/gpu/drm/drm_atomic.c:2031 at __drm_atomic_helper_set_config+0x18e/0x1b0 [drm] Call Trace: drm_client_modeset_commit_atomic+0x16b/0x220 [drm] drm_client_modeset_commit_locked+0x56/0x160 [drm] drm_client_modeset_commit+0x21/0x40 [drm] __drm_fb_helper_restore_fbdev_mode_unlocked.part.0+0x7b/0x80 drm_fbdev_client_restore+0xe/0x20 [drm_client_lib] drm_client_dev_restore+0x9f/0xc0 [drm] drm_release+0xc5/0xe0 [drm]
The warning is followed by a NULL pointer dereference:
BUG: kernel NULL pointer dereference, address: 0000000000000008
RIP:
__drm_fb_helper_restore_fbdev_mode_unlocked.part.0+0x41/0x80 [drm_kms_helper]
Call Trace: drm_fbdev_client_restore+0xe/0x20 [drm_client_lib] drm_client_dev_restore+0x9f/0xc0 [drm] drm_release+0xc5/0xe0 [drm] __fput+0xdc/0x2b0 __x64_sys_close+0x39/0x80 do_syscall_64+0x8d/0x460 entry_SYSCALL_64_after_hwframe+0x76/0x7e
drm_client_register() adds the DRM client to the device client list before invoking the initial hotplug callback. If the hotplug callback fails, the client remains registered.
For the fbdev client, a failure during drm_fb_helper_initial_config() causes the partially initialized fbdev helper to be cleaned up. drm_fb_helper_fini() releases fb_helper->info and leaves it NULL.
The fbdev client therefore remains registered even though there is no fully initialized framebuffer device.
Later, when userspace closes the DRM file descriptor, drm_release() can invoke the restore callbacks of registered DRM clients:
drm_release()drm_client_dev_restore()drm_fbdev_client_restore()drm_fb_helper_restore_fbdev_mode_unlocked()
drm_fbdev_client_restore() currently restores the fbdev state unconditionally. For a partially initialized fbdev client this can submit an incomplete modeset state and subsequently access fbdev state which has not been initialized, resulting in the warning and NULL pointer dereference above.
drm_fbdev_client_unregister() already uses fb_helper->info to distinguish a fully probed framebuffer device from a partially initialized client.
Use the same condition in drm_fbdev_client_restore() and skip restore if no framebuffer device has been successfully initialized.
Signed-off-by shechenglong <[email protected]>
Reviewed-by Thomas Zimmermann <[email protected]>
Fixes 5d08c44e47b9 ("drm/fbdev: Add memory-agnostic fbdev client")
Signed-off-by Thomas Zimmermann <[email protected]>
Cc <[email protected]> # v6.13+
Signed-off-by Sasha Levin <[email protected]>
--- drivers/gpu/drm/clients/drm_fbdev_client.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/gpu/drm/clients/drm_fbdev_client.c +8 -0
@@ -30,6 +30,14 @@ static int drm_fbdev_client_restore(struct drm_client_dev *client, bool force) {struct drm_fb_helper *fb_helper =drm_fb_helper_from_client (client); +/* +* The client is registered before the initial fbdev probe. +* If probing failed, the client remains registered but there +* is no valid fbdev framebuffer to restore. +*/ +if (!fb_helper->info || !fb_helper->fb) +return 0 ; +drm_fb_helper_restore_fbdev_mode_unlocked (fb_helper, force);return 0 ;
Signature · 1 line
2.53.0