I have a question about function sync_timeline_destroy() in 
kernel/driver/base/sync.c, this is a piece of new code added by Android to 
implement sync mechanism.
its code are as follows:

void sync_timeline_destroy(struct sync_timeline *obj)
{
obj->destroyed = true;

/*
 * If this is not the last reference, signal any children
 * that their parent is going away.
 */

if (!kref_put(&obj->kref, sync_timeline_free))
sync_timeline_signal(obj);
}

I am wondering if calling sync_timeline_signal(obj) is correct here.  
*kref_put(&obj->kref, 
sync_timeline_free)* returning 0 only means object was NOT removed. "NOT 
removed" could be due to that (1)the object having valid refcount so 
shouldn't be removed OR (2)the object has already been removed so shouldn't 
be removed again. if case (2) happens, here calling sync_timeline_signa is 
NOT correct, because sync_timeline_signal will operate on a freed memory 
then could cause kernel panic or corrupt the memory of other slab if it is 
allocated for other slab.

when case (2) will happen?
1. when refcount of timeline object is 0, calling kref_put(&obj->kref, 
sync_timeline_free) will decrease the refcount to 0xFFFFFFFF, then 
sync_timeline_free will not be called, but kref_put() will return 0.
2. when slab debug is enabled, if timeline object has been freed, the slab 
of timeline will be filled with 0x6B6B6B6B, calling kref_put(&obj->kref, 
sync_timeline_free) will not free timeline object and will return 0.

in both 2 cases above, sync_timeline_signal will be called since kref_put() 
return 0, then sync_timeline_signal will operate on a freed memory.

I think here sync_timeline_signal(obj) shouldn't be called, the signaling 
timeline should NOT depends on the operation of timeline destruction.

Here is the patch for my idea.

diff --git a/drivers/base/sync.c b/drivers/base/sync.c
index 2e35996..b3faebf 100644
--- a/drivers/base/sync.c
+++ b/drivers/base/sync.c
@@ -98,8 +98,7 @@ void sync_timeline_destroy(struct sync_timeline *obj)
         * that their parent is going away.
         */
 
-       if (!kref_put(&obj->kref, sync_timeline_free))
-               sync_timeline_signal(obj);
+       kref_put(&obj->kref, sync_timeline_free);
 }
 EXPORT_SYMBOL(sync_timeline_destroy);

-- 
-- 
unsubscribe: android-kernel+unsubscr...@googlegroups.com
website: http://groups.google.com/group/android-kernel
--- 
You received this message because you are subscribed to the Google Groups 
"Android Linux Kernel Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to android-kernel+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to