[Intel-gfx] [PATCH] drm/i915/fbdev: Check for existence of ifbdev->vma before operations

[Tvrtko Ursulin]

On 22/06/2017 17:02, Chris Wilson wrote:
> Commit fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer
> flushes") adds a dependency to ifbdev->vma when flushing the framebufer,
> but the checks are only against the existence of the ifbdev->fb and not
> against ifbdev->vma. This leaves a window of opportunity where we may
> try to operate on the fbdev prior to it being probed (thanks to
> asynchronous booting).

How about changing the intelfb_alloc to not write the fb to ifbdev but 
instead return it to the caller, so intelfb_create could then set the 
ifbdev->fb and ifbdev->vma atomically under the mutex?

Regards,

Tvrtko

> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101534
> Fixes: fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer flushes")
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter at intel.com>
> ---
>   drivers/gpu/drm/i915/intel_fbdev.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 03347c6ae599..0c4cde6b2e6f 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -535,13 +535,14 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)
>   
>   	drm_fb_helper_fini(&ifbdev->helper);
>   
> -	if (ifbdev->fb) {
> +	if (ifbdev->vma) {
>   		mutex_lock(&ifbdev->helper.dev->struct_mutex);
>   		intel_unpin_fb_vma(ifbdev->vma);
>   		mutex_unlock(&ifbdev->helper.dev->struct_mutex);
> +	}
>   
> +	if (ifbdev->fb)
>   		drm_framebuffer_remove(&ifbdev->fb->base);
> -	}
>   
>   	kfree(ifbdev);
>   }
> @@ -765,7 +766,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
>   	struct intel_fbdev *ifbdev = dev_priv->fbdev;
>   	struct fb_info *info;
>   
> -	if (!ifbdev || !ifbdev->fb)
> +	if (!ifbdev || !ifbdev->vma)
>   		return;
>   
>   	info = ifbdev->helper.fbdev;
> @@ -812,7 +813,7 @@ void intel_fbdev_output_poll_changed(struct drm_device *dev)
>   {
>   	struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
>   
> -	if (ifbdev && ifbdev->fb)
> +	if (ifbdev && ifbdev->vma)
>   		drm_fb_helper_hotplug_event(&ifbdev->helper);
>   }
>   
> @@ -824,7 +825,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev)
>   		return;
>   
>   	intel_fbdev_sync(ifbdev);
> -	if (!ifbdev->fb)
> +	if (!ifbdev->vma)
>   		return;
>   
>   	if (drm_fb_helper_restore_fbdev_mode_unlocked(&ifbdev->helper) == 0)
>