Commit 39b4d07a authored by Chris Wilson's avatar Chris Wilson Committed by Dave Airlie
Browse files

drm: Hold the mutex when dropping the last GEM reference (v2) 

In order to be fully threadsafe we need to check that the drm_gem_object
refcount is still 0 after acquiring the mutex in order to call the free
function. Otherwise, we may encounter scenarios like:

Thread A:                                        Thread B:
drm_gem_close
unreference_unlocked
kref_put                                         mutex_lock
...                                              i915_gem_evict
...                                              kref_get -> BUG
...                                              i915_gem_unbind
...                                              kref_put
...                                              i915_gem_object_free
...                                              mutex_unlock
mutex_lock
i915_gem_object_free -> BUG
i915_gem_object_unbind
kfree
mutex_unlock

Note that no driver is currently using the free_unlocked vfunc and it is
scheduled for removal, hasten that process.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=30454


Reported-and-Tested-by: default avatarMagnus Kessler <Magnus.Kessler@gmx.net>
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: stable@kernel.org
Signed-off-by: default avatarDave Airlie <airlied@redhat.com>
parent 29d08b3e

drivers/gpu/drm/drm_gem.c

+0 −22
Original line number Diff line number Diff line
@@ -462,28 +462,6 @@ drm_gem_object_free(struct kref *kref) 
}

EXPORT_SYMBOL(drm_gem_object_free);



/**

 * Called after the last reference to the object has been lost.

 * Must be called without holding struct_mutex

 *

 * Frees the object

 */

void

drm_gem_object_free_unlocked(struct kref *kref)

{

	struct drm_gem_object *obj = (struct drm_gem_object *) kref;

	struct drm_device *dev = obj->dev;



	if (dev->driver->gem_free_object_unlocked != NULL)

		dev->driver->gem_free_object_unlocked(obj);

	else if (dev->driver->gem_free_object != NULL) {

		mutex_lock(&dev->struct_mutex);

		dev->driver->gem_free_object(obj);

		mutex_unlock(&dev->struct_mutex);

	}

}

EXPORT_SYMBOL(drm_gem_object_free_unlocked);



static void drm_gem_object_ref_bug(struct kref *list_kref)

{

	BUG();

include/drm/drmP.h

+6 −4
Original line number Diff line number Diff line
@@ -808,7 +808,6 @@ struct drm_driver { 
	 */

	int (*gem_init_object) (struct drm_gem_object *obj);

	void (*gem_free_object) (struct drm_gem_object *obj);

	void (*gem_free_object_unlocked) (struct drm_gem_object *obj);



	/* vga arb irq handler */

	void (*vgaarb_irq)(struct drm_device *dev, bool state);
@@ -1456,7 +1455,6 @@ int drm_gem_init(struct drm_device *dev); 
void drm_gem_destroy(struct drm_device *dev);

void drm_gem_object_release(struct drm_gem_object *obj);

void drm_gem_object_free(struct kref *kref);

void drm_gem_object_free_unlocked(struct kref *kref);

struct drm_gem_object *drm_gem_object_alloc(struct drm_device *dev,

					    size_t size);

int drm_gem_object_init(struct drm_device *dev,
@@ -1484,8 +1482,12 @@ drm_gem_object_unreference(struct drm_gem_object *obj) 
static inline void

drm_gem_object_unreference_unlocked(struct drm_gem_object *obj)

{

	if (obj != NULL)

		kref_put(&obj->refcount, drm_gem_object_free_unlocked);

	if (obj != NULL) {

		struct drm_device *dev = obj->dev;

		mutex_lock(&dev->struct_mutex);

		kref_put(&obj->refcount, drm_gem_object_free);

		mutex_unlock(&dev->struct_mutex);

	}

}



int drm_gem_handle_create(struct drm_file *file_priv,

CRA Git | Maintained and supported by SUSTech CRA and CCSE