[Intel-gfx] [PATCH] drm: Fixup racy refcounting in plane_force_disable

[Matt Roper]

On Fri, Feb 27, 2015 at 01:03:37PM +0100, Daniel Vetter wrote:
> Originally it was impossible to be dropping the last refcount in this
> function since there was always one around still from the idr. But in
> 
> commit 83f45fc360c8e16a330474860ebda872d1384c8c
> Author: Daniel Vetter <daniel.vetter at ffwll.ch>
> Date:   Wed Aug 6 09:10:18 2014 +0200
> 
>     drm: Don't grab an fb reference for the idr
> 
> we've switched to weak references, broke that assumption but forgot to
> fix it up.
> 
> Since we still force-disable planes it's only possible to hit this
> when racing multiple rmfb with fbdev restoring or similar evil things.
> As long as userspace is nice it's impossible to hit the BUG_ON.
> 
> But the BUG_ON would most likely be hit from fbdev code, which usually
> invovles the console_lock besides all modeset locks. So very likely
> we'd never get the bug reports if this was hit in the wild, hence
> better be safe than sorry and backport.
> 
> Spotted by Matt Roper while reviewing other patches.
> 
> Cc: stable at vger.kernel.org
> Cc: Matt Roper <matthew.d.roper at intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>

> ---
>  drivers/gpu/drm/drm_crtc.c | 13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index cc548ecd3634..897f51beaadd 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -524,17 +524,6 @@ void drm_framebuffer_reference(struct drm_framebuffer *fb)
>  }
>  EXPORT_SYMBOL(drm_framebuffer_reference);
>  
> -static void drm_framebuffer_free_bug(struct kref *kref)
> -{
> -	BUG();
> -}
> -
> -static void __drm_framebuffer_unreference(struct drm_framebuffer *fb)
> -{
> -	DRM_DEBUG("%p: FB ID: %d (%d)\n", fb, fb->base.id, atomic_read(&fb->refcount.refcount));
> -	kref_put(&fb->refcount, drm_framebuffer_free_bug);
> -}
> -
>  /**
>   * drm_framebuffer_unregister_private - unregister a private fb from the lookup idr
>   * @fb: fb to unregister
> @@ -1319,7 +1308,7 @@ void drm_plane_force_disable(struct drm_plane *plane)
>  		return;
>  	}
>  	/* disconnect the plane from the fb and crtc: */
> -	__drm_framebuffer_unreference(plane->old_fb);
> +	drm_framebuffer_unreference(plane->old_fb);
>  	plane->old_fb = NULL;
>  	plane->fb = NULL;
>  	plane->crtc = NULL;
> -- 
> 2.1.4
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795