Nouveau - Feb 2023 - [PATCH] drm/gem: Expose the buffer object handle to userspace last

From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Currently drm_gem_handle_create_tail exposes the handle to userspace
before the buffer object constructions is complete. This allowing
of working against a partially constructed object, which may also be in
the process of having its creation fail, can have a range of negative
outcomes.

A lot of those will depend on what the individual drivers are doing in
their obj->funcs->open() callbacks, and also with a common failure mode
being -ENOMEM from drm_vma_node_allow.

We can make sure none of this can happen by allocating a handle last,
although with a downside that more of the function now runs under the
dev->object_name_lock.

Looking into the individual drivers open() hooks, we have
amdgpu_gem_object_open which seems like it could have a potential security
issue without this change.

A couple drivers like qxl_gem_object_open and vmw_gem_object_open
implement no-op hooks so no impact for them.

A bunch of other require a deeper look by individual owners to asses for
impact. Those are lima_gem_object_open, nouveau_gem_object_open,
panfrost_gem_open, radeon_gem_object_open and virtio_gpu_gem_object_open.

Putting aside the risk assesment of the above, some common scenarios to
think about are along these lines:

1)
Userspace closes a handle by speculatively "guessing" it from a second
thread.

This results in an unreachable buffer object so, a memory leak.

2)
Same as 1), but object is in the process of getting closed (failed
creation).

The second thread is then able to re-cycle the handle and idr_remove would
in the first thread would then remove the handle it does not own from the
idr.

3)
Going back to the earlier per driver problem space - individual impact
assesment of allowing a second thread to access and operate on a partially
constructed handle / object. (Can something crash? Leak information?)

In terms of identifying when the problem started I will tag some patches
as references, but not all, if even any, of them actually point to a
broken state. I am just identifying points at which more opportunity for
issues to arise was added.

References: 304eda32920b ("drm/gem: add hooks to notify driver when object
handle is created/destroyed")
References: ca481c9b2a3a ("drm/gem: implement vma access management")
References: b39b5394fabc ("drm/gem: Add drm_gem_object_funcs")
Cc: dri-devel at lists.freedesktop.org
Cc: Rob Clark <robdclark at chromium.org>
Cc: Ben Skeggs <bskeggs at redhat.com>
Cc: David Herrmann <dh.herrmann at gmail.com>
Cc: Noralf Tr?nnes <noralf at tronnes.org>
Cc: David Airlie <airlied at gmail.com>
Cc: Daniel Vetter <daniel at ffwll.ch>
Cc: amd-gfx at lists.freedesktop.org
Cc: lima at lists.freedesktop.org
Cc: nouveau at lists.freedesktop.org
Cc: Steven Price <steven.price at arm.com>
Cc: virtualization at lists.linux-foundation.org
Cc: spice-devel at lists.freedesktop.org
Cc: Zack Rusin <zackr at vmware.com>
---
 drivers/gpu/drm/drm_gem.c | 48 +++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index aa15c52ae182..e3d897bca0f2 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -356,52 +356,52 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
 			   u32 *handlep)
 {
 	struct drm_device *dev = obj->dev;
-	u32 handle;
 	int ret;
 
 	WARN_ON(!mutex_is_locked(&dev->object_name_lock));
 	if (obj->handle_count++ == 0)
 		drm_gem_object_get(obj);
 
+	ret = drm_vma_node_allow(&obj->vma_node, file_priv);
+	if (ret)
+		goto err_put;
+
+	if (obj->funcs->open) {
+		ret = obj->funcs->open(obj, file_priv);
+		if (ret)
+			goto err_revoke;
+	}
+
 	/*
-	 * Get the user-visible handle using idr.  Preload and perform
-	 * allocation under our spinlock.
+	 * Get the user-visible handle using idr as the _last_ step.
+	 * Preload and perform allocation under our spinlock.
 	 */
 	idr_preload(GFP_KERNEL);
 	spin_lock(&file_priv->table_lock);
-
 	ret = idr_alloc(&file_priv->object_idr, obj, 1, 0, GFP_NOWAIT);
-
 	spin_unlock(&file_priv->table_lock);
 	idr_preload_end();
 
-	mutex_unlock(&dev->object_name_lock);
 	if (ret < 0)
-		goto err_unref;
-
-	handle = ret;
+		goto err_close;
 
-	ret = drm_vma_node_allow(&obj->vma_node, file_priv);
-	if (ret)
-		goto err_remove;
+	mutex_unlock(&dev->object_name_lock);
 
-	if (obj->funcs->open) {
-		ret = obj->funcs->open(obj, file_priv);
-		if (ret)
-			goto err_revoke;
-	}
+	*handlep = ret;
 
-	*handlep = handle;
 	return 0;
 
+err_close:
+	if (obj->funcs->close)
+		obj->funcs->close(obj, file_priv);
 err_revoke:
 	drm_vma_node_revoke(&obj->vma_node, file_priv);
-err_remove:
-	spin_lock(&file_priv->table_lock);
-	idr_remove(&file_priv->object_idr, handle);
-	spin_unlock(&file_priv->table_lock);
-err_unref:
-	drm_gem_object_handle_put_unlocked(obj);
+err_put:
+	if (--obj->handle_count == 0)
+		drm_gem_object_put(obj);
+
+	mutex_unlock(&dev->object_name_lock);
+
 	return ret;
 }
 
-- 
2.34.1

Am 14.02.23 um 13:50 schrieb Tvrtko Ursulin:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> Currently drm_gem_handle_create_tail exposes the handle to userspace
> before the buffer object constructions is complete. This allowing
> of working against a partially constructed object, which may also be in
> the process of having its creation fail, can have a range of negative
> outcomes.
>
> A lot of those will depend on what the individual drivers are doing in
> their obj->funcs->open() callbacks, and also with a common failure
mode
> being -ENOMEM from drm_vma_node_allow.
>
> We can make sure none of this can happen by allocating a handle last,
> although with a downside that more of the function now runs under the
> dev->object_name_lock.
>
> Looking into the individual drivers open() hooks, we have
> amdgpu_gem_object_open which seems like it could have a potential security
> issue without this change.
>
> A couple drivers like qxl_gem_object_open and vmw_gem_object_open
> implement no-op hooks so no impact for them.
>
> A bunch of other require a deeper look by individual owners to asses for
> impact. Those are lima_gem_object_open, nouveau_gem_object_open,
> panfrost_gem_open, radeon_gem_object_open and virtio_gpu_gem_object_open.
>
> Putting aside the risk assesment of the above, some common scenarios to
> think about are along these lines:
>
> 1)
> Userspace closes a handle by speculatively "guessing" it from a
second
> thread.
>
> This results in an unreachable buffer object so, a memory leak.
>
> 2)
> Same as 1), but object is in the process of getting closed (failed
> creation).
>
> The second thread is then able to re-cycle the handle and idr_remove would
> in the first thread would then remove the handle it does not own from the
> idr.
>
> 3)
> Going back to the earlier per driver problem space - individual impact
> assesment of allowing a second thread to access and operate on a partially
> constructed handle / object. (Can something crash? Leak information?)
>
> In terms of identifying when the problem started I will tag some patches
> as references, but not all, if even any, of them actually point to a
> broken state. I am just identifying points at which more opportunity for
> issues to arise was added.
Yes I've looked into this once as well, but couldn't completely solve it
for some reason.

Give me a day or two to get this tested and all the logic swapped back 
into my head again.

Christian.
>
> References: 304eda32920b ("drm/gem: add hooks to notify driver when
object handle is created/destroyed")
> References: ca481c9b2a3a ("drm/gem: implement vma access
management")
> References: b39b5394fabc ("drm/gem: Add drm_gem_object_funcs")
> Cc: dri-devel at lists.freedesktop.org
> Cc: Rob Clark <robdclark at chromium.org>
> Cc: Ben Skeggs <bskeggs at redhat.com>
> Cc: David Herrmann <dh.herrmann at gmail.com>
> Cc: Noralf Tr?nnes <noralf at tronnes.org>
> Cc: David Airlie <airlied at gmail.com>
> Cc: Daniel Vetter <daniel at ffwll.ch>
> Cc: amd-gfx at lists.freedesktop.org
> Cc: lima at lists.freedesktop.org
> Cc: nouveau at lists.freedesktop.org
> Cc: Steven Price <steven.price at arm.com>
> Cc: virtualization at lists.linux-foundation.org
> Cc: spice-devel at lists.freedesktop.org
> Cc: Zack Rusin <zackr at vmware.com>
> ---
>   drivers/gpu/drm/drm_gem.c | 48 +++++++++++++++++++--------------------
>   1 file changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index aa15c52ae182..e3d897bca0f2 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -356,52 +356,52 @@ drm_gem_handle_create_tail(struct drm_file
*file_priv,
>   			   u32 *handlep)
>   {
>   	struct drm_device *dev = obj->dev;
> -	u32 handle;
>   	int ret;
>   
>   	WARN_ON(!mutex_is_locked(&dev->object_name_lock));
>   	if (obj->handle_count++ == 0)
>   		drm_gem_object_get(obj);
>   
> +	ret = drm_vma_node_allow(&obj->vma_node, file_priv);
> +	if (ret)
> +		goto err_put;
> +
> +	if (obj->funcs->open) {
> +		ret = obj->funcs->open(obj, file_priv);
> +		if (ret)
> +			goto err_revoke;
> +	}
> +
>   	/*
> -	 * Get the user-visible handle using idr.  Preload and perform
> -	 * allocation under our spinlock.
> +	 * Get the user-visible handle using idr as the _last_ step.
> +	 * Preload and perform allocation under our spinlock.
>   	 */
>   	idr_preload(GFP_KERNEL);
>   	spin_lock(&file_priv->table_lock);
> -
>   	ret = idr_alloc(&file_priv->object_idr, obj, 1, 0, GFP_NOWAIT);
> -
>   	spin_unlock(&file_priv->table_lock);
>   	idr_preload_end();
>   
> -	mutex_unlock(&dev->object_name_lock);
>   	if (ret < 0)
> -		goto err_unref;
> -
> -	handle = ret;
> +		goto err_close;
>   
> -	ret = drm_vma_node_allow(&obj->vma_node, file_priv);
> -	if (ret)
> -		goto err_remove;
> +	mutex_unlock(&dev->object_name_lock);
>   
> -	if (obj->funcs->open) {
> -		ret = obj->funcs->open(obj, file_priv);
> -		if (ret)
> -			goto err_revoke;
> -	}
> +	*handlep = ret;
>   
> -	*handlep = handle;
>   	return 0;
>   
> +err_close:
> +	if (obj->funcs->close)
> +		obj->funcs->close(obj, file_priv);
>   err_revoke:
>   	drm_vma_node_revoke(&obj->vma_node, file_priv);
> -err_remove:
> -	spin_lock(&file_priv->table_lock);
> -	idr_remove(&file_priv->object_idr, handle);
> -	spin_unlock(&file_priv->table_lock);
> -err_unref:
> -	drm_gem_object_handle_put_unlocked(obj);
> +err_put:
> +	if (--obj->handle_count == 0)
> +		drm_gem_object_put(obj);
> +
> +	mutex_unlock(&dev->object_name_lock);
> +
>   	return ret;
>   }
>

On 14/02/2023 12:50, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> Currently drm_gem_handle_create_tail exposes the handle to userspace
> before the buffer object constructions is complete. This allowing
> of working against a partially constructed object, which may also be in
> the process of having its creation fail, can have a range of negative
> outcomes.
> 
> A lot of those will depend on what the individual drivers are doing in
> their obj->funcs->open() callbacks, and also with a common failure
mode
> being -ENOMEM from drm_vma_node_allow.
> 
> We can make sure none of this can happen by allocating a handle last,
> although with a downside that more of the function now runs under the
> dev->object_name_lock.
> 
> Looking into the individual drivers open() hooks, we have
> amdgpu_gem_object_open which seems like it could have a potential security
> issue without this change.
> 
> A couple drivers like qxl_gem_object_open and vmw_gem_object_open
> implement no-op hooks so no impact for them.
> 
> A bunch of other require a deeper look by individual owners to asses for
> impact. Those are lima_gem_object_open, nouveau_gem_object_open,
> panfrost_gem_open, radeon_gem_object_open and virtio_gpu_gem_object_open.
I've looked over the panfrost code, and I can't see how this could
create a security hole there. It looks like there's a path which can
confuse the shrinker (so objects might not be purged when they could
be[1]) but they would be freed properly in the normal path - so no worse
than user space could already do.

[1] gpu_usecount is incremented in panfrost_lookup_bos() per bo, but not
decremented on failure.
> Putting aside the risk assesment of the above, some common scenarios to
> think about are along these lines:
> 
> 1)
> Userspace closes a handle by speculatively "guessing" it from a
second
> thread.
> 
> This results in an unreachable buffer object so, a memory leak.
> 
> 2)
> Same as 1), but object is in the process of getting closed (failed
> creation).
> 
> The second thread is then able to re-cycle the handle and idr_remove would
> in the first thread would then remove the handle it does not own from the
> idr.
This, however, looks plausible - and I can see how this could
potentially trigger a security hole in user space.
> 3)
> Going back to the earlier per driver problem space - individual impact
> assesment of allowing a second thread to access and operate on a partially
> constructed handle / object. (Can something crash? Leak information?)
> 
> In terms of identifying when the problem started I will tag some patches
> as references, but not all, if even any, of them actually point to a
> broken state. I am just identifying points at which more opportunity for
> issues to arise was added.
> 
> References: 304eda32920b ("drm/gem: add hooks to notify driver when
object handle is created/destroyed")
> References: ca481c9b2a3a ("drm/gem: implement vma access
management")
> References: b39b5394fabc ("drm/gem: Add drm_gem_object_funcs")
> Cc: dri-devel at lists.freedesktop.org
> Cc: Rob Clark <robdclark at chromium.org>
> Cc: Ben Skeggs <bskeggs at redhat.com>
> Cc: David Herrmann <dh.herrmann at gmail.com>
> Cc: Noralf Tr?nnes <noralf at tronnes.org>
> Cc: David Airlie <airlied at gmail.com>
> Cc: Daniel Vetter <daniel at ffwll.ch>
> Cc: amd-gfx at lists.freedesktop.org
> Cc: lima at lists.freedesktop.org
> Cc: nouveau at lists.freedesktop.org
> Cc: Steven Price <steven.price at arm.com>
> Cc: virtualization at lists.linux-foundation.org
> Cc: spice-devel at lists.freedesktop.org
> Cc: Zack Rusin <zackr at vmware.com>
FWIW I think this makes the code easier to reason about, so

Reviewed-by: Steven Price <steven.price at arm.com>
> ---
>  drivers/gpu/drm/drm_gem.c | 48 +++++++++++++++++++--------------------
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index aa15c52ae182..e3d897bca0f2 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -356,52 +356,52 @@ drm_gem_handle_create_tail(struct drm_file
*file_priv,
>  			   u32 *handlep)
>  {
>  	struct drm_device *dev = obj->dev;
> -	u32 handle;
>  	int ret;
>  
>  	WARN_ON(!mutex_is_locked(&dev->object_name_lock));
>  	if (obj->handle_count++ == 0)
>  		drm_gem_object_get(obj);
>  
> +	ret = drm_vma_node_allow(&obj->vma_node, file_priv);
> +	if (ret)
> +		goto err_put;
> +
> +	if (obj->funcs->open) {
> +		ret = obj->funcs->open(obj, file_priv);
> +		if (ret)
> +			goto err_revoke;
> +	}
> +
>  	/*
> -	 * Get the user-visible handle using idr.  Preload and perform
> -	 * allocation under our spinlock.
> +	 * Get the user-visible handle using idr as the _last_ step.
> +	 * Preload and perform allocation under our spinlock.
>  	 */
>  	idr_preload(GFP_KERNEL);
>  	spin_lock(&file_priv->table_lock);
> -
>  	ret = idr_alloc(&file_priv->object_idr, obj, 1, 0, GFP_NOWAIT);
> -
>  	spin_unlock(&file_priv->table_lock);
>  	idr_preload_end();
>  
> -	mutex_unlock(&dev->object_name_lock);
>  	if (ret < 0)
> -		goto err_unref;
> -
> -	handle = ret;
> +		goto err_close;
>  
> -	ret = drm_vma_node_allow(&obj->vma_node, file_priv);
> -	if (ret)
> -		goto err_remove;
> +	mutex_unlock(&dev->object_name_lock);
>  
> -	if (obj->funcs->open) {
> -		ret = obj->funcs->open(obj, file_priv);
> -		if (ret)
> -			goto err_revoke;
> -	}
> +	*handlep = ret;
>  
> -	*handlep = handle;
>  	return 0;
>  
> +err_close:
> +	if (obj->funcs->close)
> +		obj->funcs->close(obj, file_priv);
>  err_revoke:
>  	drm_vma_node_revoke(&obj->vma_node, file_priv);
> -err_remove:
> -	spin_lock(&file_priv->table_lock);
> -	idr_remove(&file_priv->object_idr, handle);
> -	spin_unlock(&file_priv->table_lock);
> -err_unref:
> -	drm_gem_object_handle_put_unlocked(obj);
> +err_put:
> +	if (--obj->handle_count == 0)
> +		drm_gem_object_put(obj);
> +
> +	mutex_unlock(&dev->object_name_lock);
> +
>  	return ret;
>  }
>

Hi,

On 14/02/2023 13:59, Christian K?nig wrote:
> Am 14.02.23 um 13:50 schrieb Tvrtko Ursulin:
>> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>>
>> Currently drm_gem_handle_create_tail exposes the handle to userspace
>> before the buffer object constructions is complete. This allowing
>> of working against a partially constructed object, which may also be in
>> the process of having its creation fail, can have a range of negative
>> outcomes.
>>
>> A lot of those will depend on what the individual drivers are doing in
>> their obj->funcs->open() callbacks, and also with a common
failure mode
>> being -ENOMEM from drm_vma_node_allow.
>>
>> We can make sure none of this can happen by allocating a handle last,
>> although with a downside that more of the function now runs under the
>> dev->object_name_lock.
>>
>> Looking into the individual drivers open() hooks, we have
>> amdgpu_gem_object_open which seems like it could have a potential 
>> security
>> issue without this change.
>>
>> A couple drivers like qxl_gem_object_open and vmw_gem_object_open
>> implement no-op hooks so no impact for them.
>>
>> A bunch of other require a deeper look by individual owners to asses
for
>> impact. Those are lima_gem_object_open, nouveau_gem_object_open,
>> panfrost_gem_open, radeon_gem_object_open and
virtio_gpu_gem_object_open.
>>
>> Putting aside the risk assesment of the above, some common scenarios to
>> think about are along these lines:
>>
>> 1)
>> Userspace closes a handle by speculatively "guessing" it from
a second
>> thread.
>>
>> This results in an unreachable buffer object so, a memory leak.
>>
>> 2)
>> Same as 1), but object is in the process of getting closed (failed
>> creation).
>>
>> The second thread is then able to re-cycle the handle and idr_remove 
>> would
>> in the first thread would then remove the handle it does not own from
the
>> idr.
>>
>> 3)
>> Going back to the earlier per driver problem space - individual impact
>> assesment of allowing a second thread to access and operate on a 
>> partially
>> constructed handle / object. (Can something crash? Leak information?)
>>
>> In terms of identifying when the problem started I will tag some
patches
>> as references, but not all, if even any, of them actually point to a
>> broken state. I am just identifying points at which more opportunity
for
>> issues to arise was added.
> 
> Yes I've looked into this once as well, but couldn't completely
solve it
> for some reason.
> 
> Give me a day or two to get this tested and all the logic swapped back 
> into my head again.
Managed to recollect what the problem with earlier attempts was?

Regards,

Tvrtko
> Christian.
> 
>>
>> References: 304eda32920b ("drm/gem: add hooks to notify driver
when
>> object handle is created/destroyed")
>> References: ca481c9b2a3a ("drm/gem: implement vma access
management")
>> References: b39b5394fabc ("drm/gem: Add
drm_gem_object_funcs")
>> Cc: dri-devel at lists.freedesktop.org
>> Cc: Rob Clark <robdclark at chromium.org>
>> Cc: Ben Skeggs <bskeggs at redhat.com>
>> Cc: David Herrmann <dh.herrmann at gmail.com>
>> Cc: Noralf Tr?nnes <noralf at tronnes.org>
>> Cc: David Airlie <airlied at gmail.com>
>> Cc: Daniel Vetter <daniel at ffwll.ch>
>> Cc: amd-gfx at lists.freedesktop.org
>> Cc: lima at lists.freedesktop.org
>> Cc: nouveau at lists.freedesktop.org
>> Cc: Steven Price <steven.price at arm.com>
>> Cc: virtualization at lists.linux-foundation.org
>> Cc: spice-devel at lists.freedesktop.org
>> Cc: Zack Rusin <zackr at vmware.com>
>> ---
>> ? drivers/gpu/drm/drm_gem.c | 48
+++++++++++++++++++--------------------
>> ? 1 file changed, 24 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
>> index aa15c52ae182..e3d897bca0f2 100644
>> --- a/drivers/gpu/drm/drm_gem.c
>> +++ b/drivers/gpu/drm/drm_gem.c
>> @@ -356,52 +356,52 @@ drm_gem_handle_create_tail(struct drm_file 
>> *file_priv,
>> ???????????????? u32 *handlep)
>> ? {
>> ????? struct drm_device *dev = obj->dev;
>> -??? u32 handle;
>> ????? int ret;
>> ????? WARN_ON(!mutex_is_locked(&dev->object_name_lock));
>> ????? if (obj->handle_count++ == 0)
>> ????????? drm_gem_object_get(obj);
>> +??? ret = drm_vma_node_allow(&obj->vma_node, file_priv);
>> +??? if (ret)
>> +??????? goto err_put;
>> +
>> +??? if (obj->funcs->open) {
>> +??????? ret = obj->funcs->open(obj, file_priv);
>> +??????? if (ret)
>> +??????????? goto err_revoke;
>> +??? }
>> +
>> ????? /*
>> -???? * Get the user-visible handle using idr.? Preload and perform
>> -???? * allocation under our spinlock.
>> +???? * Get the user-visible handle using idr as the _last_ step.
>> +???? * Preload and perform allocation under our spinlock.
>> ?????? */
>> ????? idr_preload(GFP_KERNEL);
>> ????? spin_lock(&file_priv->table_lock);
>> -
>> ????? ret = idr_alloc(&file_priv->object_idr, obj, 1, 0,
GFP_NOWAIT);
>> -
>> ????? spin_unlock(&file_priv->table_lock);
>> ????? idr_preload_end();
>> -??? mutex_unlock(&dev->object_name_lock);
>> ????? if (ret < 0)
>> -??????? goto err_unref;
>> -
>> -??? handle = ret;
>> +??????? goto err_close;
>> -??? ret = drm_vma_node_allow(&obj->vma_node, file_priv);
>> -??? if (ret)
>> -??????? goto err_remove;
>> +??? mutex_unlock(&dev->object_name_lock);
>> -??? if (obj->funcs->open) {
>> -??????? ret = obj->funcs->open(obj, file_priv);
>> -??????? if (ret)
>> -??????????? goto err_revoke;
>> -??? }
>> +??? *handlep = ret;
>> -??? *handlep = handle;
>> ????? return 0;
>> +err_close:
>> +??? if (obj->funcs->close)
>> +??????? obj->funcs->close(obj, file_priv);
>> ? err_revoke:
>> ????? drm_vma_node_revoke(&obj->vma_node, file_priv);
>> -err_remove:
>> -??? spin_lock(&file_priv->table_lock);
>> -??? idr_remove(&file_priv->object_idr, handle);
>> -??? spin_unlock(&file_priv->table_lock);
>> -err_unref:
>> -??? drm_gem_object_handle_put_unlocked(obj);
>> +err_put:
>> +??? if (--obj->handle_count == 0)
>> +??????? drm_gem_object_put(obj);
>> +
>> +??? mutex_unlock(&dev->object_name_lock);
>> +
>> ????? return ret;
>> ? }
>