diff mbox series

[v6,2/4] drm/i915/fbdev: suspend HPD before fbdev unregistration

Message ID 20220722125143.1604709-3-andrzej.hajda@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/display: stop HPD workers before display driver unregister | expand

Commit Message

Andrzej Hajda July 22, 2022, 12:51 p.m. UTC
HPD event after fbdev unregistration can cause registration of deferred
fbdev which will not be unregistered later, causing use-after-free.
To avoid it HPD handling should be suspended before fbdev unregistration.

It should fix following GPF:
[272.634530] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b6b: 0000 [#1] PREEMPT SMP NOPTI
[272.634536] CPU: 0 PID: 6030 Comm: i915_selftest Tainted: G     U            5.18.0-rc5-CI_DRM_11603-g12dccf4f5eef+ #1
[272.634541] Hardware name: Intel Corporation Raptor Lake Client Platform/RPL-S ADP-S DDR5 UDIMM CRB, BIOS RPLSFWI1.R00.2397.A01.2109300731 09/30/2021
[272.634545] RIP: 0010:fb_do_apertures_overlap.part.14+0x26/0x60
...
[272.634582] Call Trace:
[272.634583]  <TASK>
[272.634585]  do_remove_conflicting_framebuffers+0x59/0xa0
[272.634589]  remove_conflicting_framebuffers+0x2d/0xc0
[272.634592]  remove_conflicting_pci_framebuffers+0xc8/0x110
[272.634595]  drm_aperture_remove_conflicting_pci_framebuffers+0x52/0x70
[272.634604]  i915_driver_probe+0x63a/0xdd0 [i915]

Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5329
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5510
Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbdev.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Imre Deak Aug. 22, 2022, 5:10 p.m. UTC | #1
On Fri, Jul 22, 2022 at 02:51:41PM +0200, Andrzej Hajda wrote:
> HPD event after fbdev unregistration can cause registration of deferred
> fbdev which will not be unregistered later, causing use-after-free.
> To avoid it HPD handling should be suspended before fbdev unregistration.
> 
> It should fix following GPF:
> [272.634530] general protection fault, probably for non-canonical address 0x6b6b6b6b6b6b6b6b: 0000 [#1] PREEMPT SMP NOPTI
> [272.634536] CPU: 0 PID: 6030 Comm: i915_selftest Tainted: G     U            5.18.0-rc5-CI_DRM_11603-g12dccf4f5eef+ #1
> [272.634541] Hardware name: Intel Corporation Raptor Lake Client Platform/RPL-S ADP-S DDR5 UDIMM CRB, BIOS RPLSFWI1.R00.2397.A01.2109300731 09/30/2021
> [272.634545] RIP: 0010:fb_do_apertures_overlap.part.14+0x26/0x60
> ...
> [272.634582] Call Trace:
> [272.634583]  <TASK>
> [272.634585]  do_remove_conflicting_framebuffers+0x59/0xa0
> [272.634589]  remove_conflicting_framebuffers+0x2d/0xc0
> [272.634592]  remove_conflicting_pci_framebuffers+0xc8/0x110
> [272.634595]  drm_aperture_remove_conflicting_pci_framebuffers+0x52/0x70
> [272.634604]  i915_driver_probe+0x63a/0xdd0 [i915]
> 
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5329
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5510
> Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
> Reviewed-by: Arun R Murthy <arun.r.murthy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbdev.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index 221336178991f0..94ddc0f34fde64 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -573,7 +573,8 @@ void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
>  	if (!ifbdev)
>  		return;
>  
> -	cancel_work_sync(&dev_priv->fbdev_suspend_work);
> +	intel_fbdev_set_suspend(&dev_priv->drm, FBINFO_STATE_SUSPENDED, true);
> +
>  	if (!current_is_async())
>  		intel_fbdev_sync(ifbdev);
>  
> @@ -618,7 +619,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
>  	struct fb_info *info;
>  
>  	if (!ifbdev || !ifbdev->vma)
> -		return;
> +		goto unlock;

goto set_suspend; 

Reviewed-by: Imre Deak <imre.deak@intel.com>

>  
>  	info = ifbdev->helper.fbdev;
>  
> @@ -661,6 +662,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
>  	drm_fb_helper_set_suspend(&ifbdev->helper, state);
>  	console_unlock();
>  
> +unlock:
>  	intel_fbdev_hpd_set_suspend(dev_priv, state);
>  }
>  
> -- 
> 2.25.1
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 221336178991f0..94ddc0f34fde64 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -573,7 +573,8 @@  void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
 	if (!ifbdev)
 		return;
 
-	cancel_work_sync(&dev_priv->fbdev_suspend_work);
+	intel_fbdev_set_suspend(&dev_priv->drm, FBINFO_STATE_SUSPENDED, true);
+
 	if (!current_is_async())
 		intel_fbdev_sync(ifbdev);
 
@@ -618,7 +619,7 @@  void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
 	struct fb_info *info;
 
 	if (!ifbdev || !ifbdev->vma)
-		return;
+		goto unlock;
 
 	info = ifbdev->helper.fbdev;
 
@@ -661,6 +662,7 @@  void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
 	drm_fb_helper_set_suspend(&ifbdev->helper, state);
 	console_unlock();
 
+unlock:
 	intel_fbdev_hpd_set_suspend(dev_priv, state);
 }