[PATCH] driver:dma bug_fix : access freed memory

[PATCH] driver:dma bug_fix : access freed memory

[김정배]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=euc-kr, Size: 3497 bytes --]

>From 8f6aeb362d9e44f29d46ae7694cdfee4408406ce Mon Sep 17 00:00:00 2001
From: "KIM JUGNBAE" <jb09.kim@samsung.com>
Date: Thu, 26 Nov 2015 16:28:47 +0900
Subject: [PATCH] bug_fix : access freed memory

sync_fenc_free & fence_check_cb_func would be executed at
other cpu. fence_check_cb_func access freed fence memory after
kfree(fence) at sync_fence_free.
To escaped this issue, atomic_read(&fence->status) need to be
protected by child_list_lock.

Signed-off-by: "kimjungbae\" " <jb09.kim@samsung.com>"
---
 drivers/dma-buf/fence.c              |   13 +++++++++++++
 drivers/staging/android/sync.c       |   10 +++++++---
 drivers/staging/android/sync_debug.c |    2 ++
 include/linux/fence.h                |    1 +
 4 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c
index 7bb9d65..d40211e 100644
--- a/drivers/dma-buf/fence.c
+++ b/drivers/dma-buf/fence.c
@@ -310,6 +310,19 @@ fence_remove_callback(struct fence *fence, struct fence_cb *cb)
 }
 EXPORT_SYMBOL(fence_remove_callback);
 
+bool
+fence_remove_callback_locked(struct fence *fence, struct fence_cb *cb)
+{
+	bool ret;
+
+	ret = !list_empty(&cb->node);
+	if (ret)
+		list_del_init(&cb->node);
+
+	return ret;
+}
+EXPORT_SYMBOL(fence_remove_callback_locked);
+
 struct default_wait_cb {
 	struct fence_cb base;
 	struct task_struct *task;
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 83f640f..9b60542 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -518,12 +518,16 @@ static const struct fence_ops android_fence_ops = {
 static void sync_fence_free(struct kref *kref)
 {
 	struct sync_fence *fence = container_of(kref, struct sync_fence, kref);
-	int i, status = atomic_read(&fence->status);
+	int i;
+	unsigned long flags;
 
 	for (i = 0; i < fence->num_fences; ++i) {
-		if (status)
-			fence_remove_callback(fence->cbs[i].sync_pt,
+		spin_lock_irqsave(fence->cbs[i].sync_pt->lock, flags);
+		if (atomic_read(&fence->status))
+			fence_remove_callback_locked(fence->cbs[i].sync_pt,
 					      &fence->cbs[i].cb);
+		spin_unlock_irqrestore(fence->cbs[i].sync_pt->lock, flags);
+
 		fence_put(fence->cbs[i].sync_pt);
 	}
 
diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c
index 0153cd2..e854f35 100644
--- a/drivers/staging/android/sync_debug.c
+++ b/drivers/staging/android/sync_debug.c
@@ -155,7 +155,9 @@ static void sync_print_fence(struct seq_file *s, struct sync_fence *fence)
 			container_of(fence->cbs[i].sync_pt,
 				     struct sync_pt, base);
 
+		spin_lock_irqsave(pt->base.lock, flags);
 		sync_print_pt(s, pt, true);
+		spin_unlock_irqrestore(pt->base.lock, flags);
 	}
 
 	spin_lock_irqsave(&fence->wq.lock, flags);
diff --git a/include/linux/fence.h b/include/linux/fence.h
index d174585..a5a3fd1 100644
--- a/include/linux/fence.h
+++ b/include/linux/fence.h
@@ -224,6 +224,7 @@ signed long fence_default_wait(struct fence *fence, bool intr, signed long timeo
 int fence_add_callback(struct fence *fence, struct fence_cb *cb,
 		       fence_func_t func);
 bool fence_remove_callback(struct fence *fence, struct fence_cb *cb);
+bool fence_remove_callback_locked(struct fence *fence, struct fence_cb *cb);
 void fence_enable_sw_signaling(struct fence *fence);
 
 /**
-- 
1.7.9.5

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH] driver:dma bug_fix : access freed memory

[Sudip Mukherjee]

On Fri, Nov 27, 2015 at 05:44:53AM +0000, 김정배 wrote:
> From 8f6aeb362d9e44f29d46ae7694cdfee4408406ce Mon Sep 17 00:00:00 2001
> From: "KIM JUGNBAE" <jb09.kim@samsung.com>
> Date: Thu, 26 Nov 2015 16:28:47 +0900
> Subject: [PATCH] bug_fix : access freed memory

This part should not be present in the patch.

> 
> sync_fenc_free & fence_check_cb_func would be executed at
> other cpu. fence_check_cb_func access freed fence memory after
> kfree(fence) at sync_fence_free.
> To escaped this issue, atomic_read(&fence->status) need to be
> protected by child_list_lock.
> 
> Signed-off-by: "kimjungbae\" " <jb09.kim@samsung.com>"

The From name and the Signed-off-by name shouls be same. Mayvbe you can
consider having Kim Jugnbae (or Jungbae) as both.

> ---
>  drivers/dma-buf/fence.c              |   13 +++++++++++++
>  drivers/staging/android/sync.c       |   10 +++++++---
>  drivers/staging/android/sync_debug.c |    2 ++
>  include/linux/fence.h                |    1 +
>  4 files changed, 23 insertions(+), 3 deletions(-)

Usually staging patches can not touch anything outside staging.

regards
sudip