All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/nouveau/svm: Fix refcount leak bug and missing check against null bug
@ 2021-09-07 12:26 ` Chenyuan Mi
  0 siblings, 0 replies; 5+ messages in thread
From: Chenyuan Mi @ 2021-09-07 12:26 UTC (permalink / raw)
  Cc: yuanxzhang, Chenyuan Mi, Xiyu Yang, Xin Tan, Ben Skeggs,
	David Airlie, Daniel Vetter, dri-devel, nouveau, linux-kernel

The reference counting issue happens in one exception handling path of
nouveau_svmm_bind(). When cli->svm.svmm is null, the function forgets
to decrease the refcount of mm increased by get_task_mm(), causing a
refcount leak.

Fix this issue by using mmput() to decrease the refcount in the
exception handling path.

Also, the function forgets to do check against null when get mm
by get_task_mm().

Fix this issue by adding null check after get mm by get_task_mm().

Signed-off-by: Chenyuan Mi <cymi20@fudan.edu.cn>
Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_svm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
index b0c3422cb01f..9985bfde015a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -162,10 +162,14 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
 	 */
 
 	mm = get_task_mm(current);
+	if (!mm) {
+		return -EINVAL;
+	}
 	mmap_read_lock(mm);
 
 	if (!cli->svm.svmm) {
 		mmap_read_unlock(mm);
+		mmput(mm);
 		return -EINVAL;
 	}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH] drm/nouveau/svm: Fix refcount leak bug and missing check against null bug
@ 2021-09-07 12:26 ` Chenyuan Mi
  0 siblings, 0 replies; 5+ messages in thread
From: Chenyuan Mi @ 2021-09-07 12:26 UTC (permalink / raw)
  Cc: yuanxzhang, Chenyuan Mi, Xiyu Yang, Xin Tan, Ben Skeggs,
	David Airlie, Daniel Vetter, dri-devel, nouveau, linux-kernel

The reference counting issue happens in one exception handling path of
nouveau_svmm_bind(). When cli->svm.svmm is null, the function forgets
to decrease the refcount of mm increased by get_task_mm(), causing a
refcount leak.

Fix this issue by using mmput() to decrease the refcount in the
exception handling path.

Also, the function forgets to do check against null when get mm
by get_task_mm().

Fix this issue by adding null check after get mm by get_task_mm().

Signed-off-by: Chenyuan Mi <cymi20@fudan.edu.cn>
Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_svm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
index b0c3422cb01f..9985bfde015a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -162,10 +162,14 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
 	 */
 
 	mm = get_task_mm(current);
+	if (!mm) {
+		return -EINVAL;
+	}
 	mmap_read_lock(mm);
 
 	if (!cli->svm.svmm) {
 		mmap_read_unlock(mm);
+		mmput(mm);
 		return -EINVAL;
 	}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [Nouveau] [PATCH] drm/nouveau/svm: Fix refcount leak bug and missing check against null bug
@ 2021-09-07 12:26 ` Chenyuan Mi
  0 siblings, 0 replies; 5+ messages in thread
From: Chenyuan Mi @ 2021-09-07 12:26 UTC (permalink / raw)
  Cc: yuanxzhang, Chenyuan Mi, Xiyu Yang, Xin Tan, Ben Skeggs,
	David Airlie, Daniel Vetter, dri-devel, nouveau, linux-kernel

The reference counting issue happens in one exception handling path of
nouveau_svmm_bind(). When cli->svm.svmm is null, the function forgets
to decrease the refcount of mm increased by get_task_mm(), causing a
refcount leak.

Fix this issue by using mmput() to decrease the refcount in the
exception handling path.

Also, the function forgets to do check against null when get mm
by get_task_mm().

Fix this issue by adding null check after get mm by get_task_mm().

Signed-off-by: Chenyuan Mi <cymi20@fudan.edu.cn>
Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_svm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
index b0c3422cb01f..9985bfde015a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -162,10 +162,14 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
 	 */
 
 	mm = get_task_mm(current);
+	if (!mm) {
+		return -EINVAL;
+	}
 	mmap_read_lock(mm);
 
 	if (!cli->svm.svmm) {
 		mmap_read_unlock(mm);
+		mmput(mm);
 		return -EINVAL;
 	}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/nouveau/svm: Fix refcount leak bug and missing check against null bug
  2021-09-07 12:26 ` Chenyuan Mi
@ 2021-10-05 21:27   ` Karol Herbst
  -1 siblings, 0 replies; 5+ messages in thread
From: Karol Herbst @ 2021-10-05 21:27 UTC (permalink / raw)
  To: Chenyuan Mi
  Cc: yuanxzhang, Xiyu Yang, Xin Tan, Ben Skeggs, David Airlie,
	Daniel Vetter, dri-devel, nouveau, LKML

I think it makes sense to add a Fixes tag to this:

Fixes: 822cab6150d3 ("drm/nouveau/svm: check for SVM initialized
before migrating")
Reviewed-by: Karol Herbst <kherbst@redhat.com>

On Tue, Sep 7, 2021 at 3:20 PM Chenyuan Mi <cymi20@fudan.edu.cn> wrote:
>
> The reference counting issue happens in one exception handling path of
> nouveau_svmm_bind(). When cli->svm.svmm is null, the function forgets
> to decrease the refcount of mm increased by get_task_mm(), causing a
> refcount leak.
>
> Fix this issue by using mmput() to decrease the refcount in the
> exception handling path.
>
> Also, the function forgets to do check against null when get mm
> by get_task_mm().
>
> Fix this issue by adding null check after get mm by get_task_mm().
>
> Signed-off-by: Chenyuan Mi <cymi20@fudan.edu.cn>
> Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
> Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_svm.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index b0c3422cb01f..9985bfde015a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -162,10 +162,14 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
>          */
>
>         mm = get_task_mm(current);
> +       if (!mm) {
> +               return -EINVAL;
> +       }
>         mmap_read_lock(mm);
>
>         if (!cli->svm.svmm) {
>                 mmap_read_unlock(mm);
> +               mmput(mm);
>                 return -EINVAL;
>         }
>
> --
> 2.17.1
>


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Nouveau] [PATCH] drm/nouveau/svm: Fix refcount leak bug and missing check against null bug
@ 2021-10-05 21:27   ` Karol Herbst
  0 siblings, 0 replies; 5+ messages in thread
From: Karol Herbst @ 2021-10-05 21:27 UTC (permalink / raw)
  To: Chenyuan Mi
  Cc: yuanxzhang, Xiyu Yang, Xin Tan, Ben Skeggs, David Airlie,
	Daniel Vetter, dri-devel, nouveau, LKML

I think it makes sense to add a Fixes tag to this:

Fixes: 822cab6150d3 ("drm/nouveau/svm: check for SVM initialized
before migrating")
Reviewed-by: Karol Herbst <kherbst@redhat.com>

On Tue, Sep 7, 2021 at 3:20 PM Chenyuan Mi <cymi20@fudan.edu.cn> wrote:
>
> The reference counting issue happens in one exception handling path of
> nouveau_svmm_bind(). When cli->svm.svmm is null, the function forgets
> to decrease the refcount of mm increased by get_task_mm(), causing a
> refcount leak.
>
> Fix this issue by using mmput() to decrease the refcount in the
> exception handling path.
>
> Also, the function forgets to do check against null when get mm
> by get_task_mm().
>
> Fix this issue by adding null check after get mm by get_task_mm().
>
> Signed-off-by: Chenyuan Mi <cymi20@fudan.edu.cn>
> Signed-off-by: Xiyu Yang <xiyuyang19@fudan.edu.cn>
> Signed-off-by: Xin Tan <tanxin.ctf@gmail.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_svm.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index b0c3422cb01f..9985bfde015a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -162,10 +162,14 @@ nouveau_svmm_bind(struct drm_device *dev, void *data,
>          */
>
>         mm = get_task_mm(current);
> +       if (!mm) {
> +               return -EINVAL;
> +       }
>         mmap_read_lock(mm);
>
>         if (!cli->svm.svmm) {
>                 mmap_read_unlock(mm);
> +               mmput(mm);
>                 return -EINVAL;
>         }
>
> --
> 2.17.1
>


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-10-05 21:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-07 12:26 [PATCH] drm/nouveau/svm: Fix refcount leak bug and missing check against null bug Chenyuan Mi
2021-09-07 12:26 ` [Nouveau] " Chenyuan Mi
2021-09-07 12:26 ` Chenyuan Mi
2021-10-05 21:27 ` Karol Herbst
2021-10-05 21:27   ` [Nouveau] " Karol Herbst

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.