[Intel-gfx] [PATCH] drm: Fix race when checking for fb in the generic kms obj lookup

[Chris Wilson]

On Thu, Jul 24, 2014 at 09:54:27AM +0200, Daniel Vetter wrote:
> In my review of
> 
> commit 98f75de40e9d83c3a90d294b8fd25fa2874212a9
> Author: Rob Clark <robdclark at gmail.com>
> Date:   Fri May 30 11:37:03 2014 -0400
> 
>     drm: add object property typ
> 
> I asked for a check to make sure that we never leak an fb from the
> generic mode object lookup since those have completely different
> lifetime rules. Rob added it, but outside of the idr mutex, which
> means that our dereference of obj->type can already chase free'd
> memory.
> 
> Somehow I didn't spot this, so fix this asap.
> 
> Cc: Rob Clark <robdclark at gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/drm_crtc.c      | 6 +++---
>  drivers/gpu/drm/drm_fb_helper.c | 1 +
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f0a777747907..853ab9cad071 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -429,6 +429,9 @@ static struct drm_mode_object *_object_find(struct drm_device *dev,
>  	if (!obj || (type != DRM_MODE_OBJECT_ANY && obj->type != type) ||
>  	    (obj->id != id))
>  		obj = NULL;
> +	/* don't leak out unref'd fb's */
> +	if (obj && (obj->type == DRM_MODE_OBJECT_FB))
> +		obj = NULL;

-       if (!obj || (type != DRM_MODE_OBJECT_ANY && obj->type != type) ||
-           (obj->id != id))
+       if (obj && obj->type != type && type != DRM_MODE_OBJECT_ANY)
+               obj = NULL;
+       if (obj && WARN_ON(obj->id != id))
+               obj = NULL;
+       if (obj && WARN_ON(obj->type == DRM_MODE_OBJECT_FB))
                obj = NULL;

To break the checks up into simple steps and show that they are unlikely
errors?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre