Re: [PATCH] drm/client: fix restore of partially initialized client

[Thomas Zimmermann <tzimmermann@suse.de>]

Am 07.09.26 um 05:51 schrieb shechenglong:
> 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 <shechenglong@xfusion.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Thanks for fixing this bug.

Fixes: 5d08c44e47b9 ("drm/fbdev: Add memory-agnostic fbdev client")
Cc: <stable@vger.kernel.org> # v6.13+


> ---
>   drivers/gpu/drm/clients/drm_fbdev_client.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/clients/drm_fbdev_client.c b/drivers/gpu/drm/clients/drm_fbdev_client.c
> index 91d196a397cf..1c16bc1084c4 100644
> --- a/drivers/gpu/drm/clients/drm_fbdev_client.c
> +++ b/drivers/gpu/drm/clients/drm_fbdev_client.c
> @@ -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;

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)