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

[Rob Clark]

On Thu, Jul 24, 2014 at 6:12 AM, Daniel Vetter <daniel.vetter at ffwll.ch> 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.
>
> v2: Simplify the conditionals as suggested by Chris.
>
> Cc: Rob Clark <robdclark at gmail.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>

Reviewed-by: Rob Clark <robdclark at gmail.com>

> ---
>  drivers/gpu/drm/drm_crtc.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index f0a777747907..d87df8836aa5 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -426,8 +426,12 @@ static struct drm_mode_object *_object_find(struct drm_device *dev,
>
>         mutex_lock(&dev->mode_config.idr_mutex);
>         obj = idr_find(&dev->mode_config.crtc_idr, id);
> -       if (!obj || (type != DRM_MODE_OBJECT_ANY && obj->type != type) ||
> -           (obj->id != id))
> +       if (obj && type != DRM_MODE_OBJECT_ANY && obj->type != type)
> +               obj = NULL;
> +       if (obj && obj->id != id)
> +               obj = NULL;
> +       /* don't leak out unref'd fb's */
> +       if (obj && (obj->type == DRM_MODE_OBJECT_FB))
>                 obj = NULL;
>         mutex_unlock(&dev->mode_config.idr_mutex);
>
> @@ -454,9 +458,6 @@ struct drm_mode_object *drm_mode_object_find(struct drm_device *dev,
>          * function.*/
>         WARN_ON(type == DRM_MODE_OBJECT_FB);
>         obj = _object_find(dev, id, type);
> -       /* don't leak out unref'd fb's */
> -       if (obj && (obj->type == DRM_MODE_OBJECT_FB))
> -               obj = NULL;
>         return obj;
>  }
>  EXPORT_SYMBOL(drm_mode_object_find);
> --
> 2.0.1
>