Details

[Home]

Issue of the Implementation # L0005

Brief

drivers/gpu/drm/drm_gem.c: Potential BUG_ON assertion fails in drm_gem_object_free

Detailed Description

drm_gem_object_free() (drivers/gpu/drm/drm_gem.c) requires dev->struct_mutex to be locked before a call.

> drm_gem_object_free(struct kref *kref)
> {
>     struct drm_gem_object *obj = (struct drm_gem_object *) kref;
>     struct drm_device *dev = obj->dev;
> 
>     BUG_ON(!mutex_is_locked(&dev->struct_mutex));
> ...
drm_gem_object_free is mostly called through drm_gem_object_unreference(include/drm/drmP.h).
> static inline void
> drm_gem_object_unreference(struct drm_gem_object *obj)
> {
>         if (obj == NULL)
>                 return;
> 
>         kref_put(&obj->refcount, drm_gem_object_free);
> }
The potential issues may come from the following sources.
  1. i915_gem_set_tiling (drivers/gpu/drm/i915/i915_gem_tiling.c)
    > int
    > i915_gem_set_tiling(struct drm_device *dev, void *data,
    >            struct drm_file *file_priv)
    > {
    >         struct drm_i915_gem_set_tiling *args = data;
    >         drm_i915_private_t *dev_priv = dev->dev_private;
    >         struct drm_gem_object *obj;
    >         struct drm_i915_gem_object *obj_priv;
    >
    >         obj = drm_gem_object_lookup(dev, file_priv, args->handle);
    >         if (obj == NULL)
    >             return -EINVAL;
    >         obj_priv = obj->driver_private;
    >
    >         if (!i915_tiling_ok(dev, args->stride, obj->size, args->tiling_mode)) {
    >               drm_gem_object_unreference(obj);  <----- ISSUE #1: unreference before unlock
    >               return -EINVAL;
    >         }
    >
    >         mutex_lock(&dev->struct_mutex);
    >         ...
    line 317:
    >               ret = i915_gem_object_unbind(obj);
    >               if (ret != 0) {
    >                       WARN(ret != -ERESTARTSYS,
    >                            "failed to unbind object for tiling switch");
    >                       args->tiling_mode = obj_priv->tiling_mode;
    >                       mutex_unlock(&dev->struct_mutex);
    >                       drm_gem_object_unreference(obj);  <----- ISSUE #2: unreference after unlock
    >
    >                       return ret;
    
  2. i915_gem_pread_ioctl and i915_gem_pwrite_ioctl (drivers/gpu/drm/i915/i915_gem.c)
    > int
    > i915_gem_pread_ioctl(struct drm_device *dev, void *data,
    >                      struct drm_file *file_priv)
    > {
    >       struct drm_i915_gem_pread *args = data;
    >       struct drm_gem_object *obj;
    >       struct drm_i915_gem_object *obj_priv;
    >       int ret;
    >
    >       obj = drm_gem_object_lookup(dev, file_priv, args->handle);
    >       if (obj == NULL)
    >               return -EBADF;
    >       obj_priv = obj->driver_private;
    >
    >       /* Bounds check source.
    >        *
    >        * XXX: This could use review for overflow issues...
    >        */
    >       if (args->offset > obj->size || args->size > obj->size ||
    >           args->offset + args->size > obj->size) {
    >               drm_gem_object_unreference(obj);  <----- ISSUE #3: unreference without lock
    >               return -EINVAL;
    >       }
    >
    >       if (i915_gem_object_needs_bit17_swizzle(obj)) {
    >               ret = i915_gem_shmem_pread_slow(dev, obj, args, file_priv);
    >       } else {
    >               ret = i915_gem_shmem_pread_fast(dev, obj, args, file_priv);
    >               if (ret != 0)
    >                       ret = i915_gem_shmem_pread_slow(dev, obj, args,
    >                                                       file_priv);
    >       }
    >
    >       drm_gem_object_unreference(obj);  <----- ISSUE #4: unreference without lock
    >
    >       return ret;
    > }
    
The same is for i915_gem_pwrite_ioctl().

Component

linux-kernel 2.6.30-rc4

Accepted

http://bugzilla.kernel.org/show_bug.cgi?id=13227
commit

Status

Fixed in kernel 2.6.34-rc1

[Home]