All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: Fix gem obj imbalance due to calling drm_gem_object_put() twice
@ 2021-12-21 11:38 ` Roberto Sassu
  0 siblings, 0 replies; 4+ messages in thread
From: Roberto Sassu @ 2021-12-21 11:38 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, tzimmermann, airlied, daniel
  Cc: dri-devel, linux-kernel, Roberto Sassu, stable,
	syzbot+c8ae65286134dd1b800d

After commit 9786b65bc61ac ("drm/ttm: fix mmap refcounting"),
drm_gem_mmap_obj() takes a reference of the passed drm_gem_object at the
beginning of the function to safely dereference the mmap offset pointer,
and releases it at the end, if an error occurred. However, the cma and
shmem helpers are also releasing that reference in case of an error,
which causes the imbalance of the reference counter and the panic
reported by syzbot.

Don't release the reference in drm_gem_mmap_obj() if the mmap method was
called and it returned an error, and uniformly apply the same behavior of
the cma and shmem helpers to the ttm helper (release the reference in the
helper, not in the caller, when an error occurs).

Cc: stable@vger.kernel.org
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reported-by: syzbot+c8ae65286134dd1b800d@syzkaller.appspotmail.com
Fixes: 9786b65bc61ac ("drm/ttm: fix mmap refcounting")
---
 drivers/gpu/drm/drm_gem.c            | 3 ++-
 drivers/gpu/drm/drm_gem_ttm_helper.c | 4 +---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 4dcdec6487bb..7264a1a7a8d2 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1049,8 +1049,9 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
 
 	if (obj->funcs->mmap) {
 		ret = obj->funcs->mmap(obj, vma);
+		/* All helpers call drm_gem_object_put() */
 		if (ret)
-			goto err_drm_gem_object_put;
+			return ret;
 		WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));
 	} else {
 		if (!vma->vm_ops) {
diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c b/drivers/gpu/drm/drm_gem_ttm_helper.c
index ecf3d2a54a98..c44bfdbb722d 100644
--- a/drivers/gpu/drm/drm_gem_ttm_helper.c
+++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
@@ -101,8 +101,6 @@ int drm_gem_ttm_mmap(struct drm_gem_object *gem,
 	int ret;
 
 	ret = ttm_bo_mmap_obj(vma, bo);
-	if (ret < 0)
-		return ret;
 
 	/*
 	 * ttm has its own object refcounting, so drop gem reference
@@ -110,7 +108,7 @@ int drm_gem_ttm_mmap(struct drm_gem_object *gem,
 	 */
 	drm_gem_object_put(gem);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL(drm_gem_ttm_mmap);
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH] drm: Fix gem obj imbalance due to calling drm_gem_object_put() twice
@ 2021-12-21 11:38 ` Roberto Sassu
  0 siblings, 0 replies; 4+ messages in thread
From: Roberto Sassu @ 2021-12-21 11:38 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, tzimmermann, airlied, daniel
  Cc: stable, syzbot+c8ae65286134dd1b800d, Roberto Sassu, linux-kernel,
	dri-devel

After commit 9786b65bc61ac ("drm/ttm: fix mmap refcounting"),
drm_gem_mmap_obj() takes a reference of the passed drm_gem_object at the
beginning of the function to safely dereference the mmap offset pointer,
and releases it at the end, if an error occurred. However, the cma and
shmem helpers are also releasing that reference in case of an error,
which causes the imbalance of the reference counter and the panic
reported by syzbot.

Don't release the reference in drm_gem_mmap_obj() if the mmap method was
called and it returned an error, and uniformly apply the same behavior of
the cma and shmem helpers to the ttm helper (release the reference in the
helper, not in the caller, when an error occurs).

Cc: stable@vger.kernel.org
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reported-by: syzbot+c8ae65286134dd1b800d@syzkaller.appspotmail.com
Fixes: 9786b65bc61ac ("drm/ttm: fix mmap refcounting")
---
 drivers/gpu/drm/drm_gem.c            | 3 ++-
 drivers/gpu/drm/drm_gem_ttm_helper.c | 4 +---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 4dcdec6487bb..7264a1a7a8d2 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -1049,8 +1049,9 @@ int drm_gem_mmap_obj(struct drm_gem_object *obj, unsigned long obj_size,
 
 	if (obj->funcs->mmap) {
 		ret = obj->funcs->mmap(obj, vma);
+		/* All helpers call drm_gem_object_put() */
 		if (ret)
-			goto err_drm_gem_object_put;
+			return ret;
 		WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));
 	} else {
 		if (!vma->vm_ops) {
diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c b/drivers/gpu/drm/drm_gem_ttm_helper.c
index ecf3d2a54a98..c44bfdbb722d 100644
--- a/drivers/gpu/drm/drm_gem_ttm_helper.c
+++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
@@ -101,8 +101,6 @@ int drm_gem_ttm_mmap(struct drm_gem_object *gem,
 	int ret;
 
 	ret = ttm_bo_mmap_obj(vma, bo);
-	if (ret < 0)
-		return ret;
 
 	/*
 	 * ttm has its own object refcounting, so drop gem reference
@@ -110,7 +108,7 @@ int drm_gem_ttm_mmap(struct drm_gem_object *gem,
 	 */
 	drm_gem_object_put(gem);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL(drm_gem_ttm_mmap);
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* RE: [PATCH] drm: Fix gem obj imbalance due to calling drm_gem_object_put() twice
  2021-12-21 11:38 ` Roberto Sassu
@ 2022-01-07 10:34   ` Roberto Sassu
  -1 siblings, 0 replies; 4+ messages in thread
From: Roberto Sassu @ 2022-01-07 10:34 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, tzimmermann, airlied, daniel
  Cc: dri-devel, linux-kernel, stable, syzbot+c8ae65286134dd1b800d

> From: Roberto Sassu
> Sent: Tuesday, December 21, 2021 12:39 PM
> After commit 9786b65bc61ac ("drm/ttm: fix mmap refcounting"),
> drm_gem_mmap_obj() takes a reference of the passed drm_gem_object at the
> beginning of the function to safely dereference the mmap offset pointer,
> and releases it at the end, if an error occurred. However, the cma and
> shmem helpers are also releasing that reference in case of an error,
> which causes the imbalance of the reference counter and the panic
> reported by syzbot.
> 
> Don't release the reference in drm_gem_mmap_obj() if the mmap method was
> called and it returned an error, and uniformly apply the same behavior of
> the cma and shmem helpers to the ttm helper (release the reference in the
> helper, not in the caller, when an error occurs).

Hello

I'm wondering if this patch and:

https://lore.kernel.org/dri-devel/20211213183122.838119-1-roberto.sassu@huawei.com/

could be useful.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> Cc: stable@vger.kernel.org
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Reported-by: syzbot+c8ae65286134dd1b800d@syzkaller.appspotmail.com
> Fixes: 9786b65bc61ac ("drm/ttm: fix mmap refcounting")
> ---
>  drivers/gpu/drm/drm_gem.c            | 3 ++-
>  drivers/gpu/drm/drm_gem_ttm_helper.c | 4 +---
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 4dcdec6487bb..7264a1a7a8d2 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1049,8 +1049,9 @@ int drm_gem_mmap_obj(struct drm_gem_object
> *obj, unsigned long obj_size,
> 
>  	if (obj->funcs->mmap) {
>  		ret = obj->funcs->mmap(obj, vma);
> +		/* All helpers call drm_gem_object_put() */
>  		if (ret)
> -			goto err_drm_gem_object_put;
> +			return ret;
>  		WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));
>  	} else {
>  		if (!vma->vm_ops) {
> diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c
> b/drivers/gpu/drm/drm_gem_ttm_helper.c
> index ecf3d2a54a98..c44bfdbb722d 100644
> --- a/drivers/gpu/drm/drm_gem_ttm_helper.c
> +++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
> @@ -101,8 +101,6 @@ int drm_gem_ttm_mmap(struct drm_gem_object *gem,
>  	int ret;
> 
>  	ret = ttm_bo_mmap_obj(vma, bo);
> -	if (ret < 0)
> -		return ret;
> 
>  	/*
>  	 * ttm has its own object refcounting, so drop gem reference
> @@ -110,7 +108,7 @@ int drm_gem_ttm_mmap(struct drm_gem_object *gem,
>  	 */
>  	drm_gem_object_put(gem);
> 
> -	return 0;
> +	return ret;
>  }
>  EXPORT_SYMBOL(drm_gem_ttm_mmap);
> 
> --
> 2.32.0


^ permalink raw reply	[flat|nested] 4+ messages in thread
* RE: [PATCH] drm: Fix gem obj imbalance due to calling drm_gem_object_put() twice
@ 2022-01-07 10:34   ` Roberto Sassu
  0 siblings, 0 replies; 4+ messages in thread
From: Roberto Sassu @ 2022-01-07 10:34 UTC (permalink / raw)
  To: maarten.lankhorst, mripard, tzimmermann, airlied, daniel
  Cc: stable, syzbot+c8ae65286134dd1b800d, linux-kernel, dri-devel

> From: Roberto Sassu
> Sent: Tuesday, December 21, 2021 12:39 PM
> After commit 9786b65bc61ac ("drm/ttm: fix mmap refcounting"),
> drm_gem_mmap_obj() takes a reference of the passed drm_gem_object at the
> beginning of the function to safely dereference the mmap offset pointer,
> and releases it at the end, if an error occurred. However, the cma and
> shmem helpers are also releasing that reference in case of an error,
> which causes the imbalance of the reference counter and the panic
> reported by syzbot.
> 
> Don't release the reference in drm_gem_mmap_obj() if the mmap method was
> called and it returned an error, and uniformly apply the same behavior of
> the cma and shmem helpers to the ttm helper (release the reference in the
> helper, not in the caller, when an error occurs).

Hello

I'm wondering if this patch and:

https://lore.kernel.org/dri-devel/20211213183122.838119-1-roberto.sassu@huawei.com/

could be useful.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> Cc: stable@vger.kernel.org
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Reported-by: syzbot+c8ae65286134dd1b800d@syzkaller.appspotmail.com
> Fixes: 9786b65bc61ac ("drm/ttm: fix mmap refcounting")
> ---
>  drivers/gpu/drm/drm_gem.c            | 3 ++-
>  drivers/gpu/drm/drm_gem_ttm_helper.c | 4 +---
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 4dcdec6487bb..7264a1a7a8d2 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -1049,8 +1049,9 @@ int drm_gem_mmap_obj(struct drm_gem_object
> *obj, unsigned long obj_size,
> 
>  	if (obj->funcs->mmap) {
>  		ret = obj->funcs->mmap(obj, vma);
> +		/* All helpers call drm_gem_object_put() */
>  		if (ret)
> -			goto err_drm_gem_object_put;
> +			return ret;
>  		WARN_ON(!(vma->vm_flags & VM_DONTEXPAND));
>  	} else {
>  		if (!vma->vm_ops) {
> diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c
> b/drivers/gpu/drm/drm_gem_ttm_helper.c
> index ecf3d2a54a98..c44bfdbb722d 100644
> --- a/drivers/gpu/drm/drm_gem_ttm_helper.c
> +++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
> @@ -101,8 +101,6 @@ int drm_gem_ttm_mmap(struct drm_gem_object *gem,
>  	int ret;
> 
>  	ret = ttm_bo_mmap_obj(vma, bo);
> -	if (ret < 0)
> -		return ret;
> 
>  	/*
>  	 * ttm has its own object refcounting, so drop gem reference
> @@ -110,7 +108,7 @@ int drm_gem_ttm_mmap(struct drm_gem_object *gem,
>  	 */
>  	drm_gem_object_put(gem);
> 
> -	return 0;
> +	return ret;
>  }
>  EXPORT_SYMBOL(drm_gem_ttm_mmap);
> 
> --
> 2.32.0


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-01-07 10:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-21 11:38 [PATCH] drm: Fix gem obj imbalance due to calling drm_gem_object_put() twice Roberto Sassu
2021-12-21 11:38 ` Roberto Sassu
2022-01-07 10:34 ` Roberto Sassu
2022-01-07 10:34   ` Roberto Sassu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.