Re: Re: [PATCH] fix race condition between device_del and device_add

Re: Re: [PATCH] fix race condition between device_del and device_add

[EunTaik Lee]

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


>
>The bus that the device is on should prevent this, why isn't that
>working for you?  What type of device/bus do you see this problem on?

The device is a firmware class device on a virtual bus.

>> Signed-off-by: eun.taik.lee 
>
>We need a "real" name here, I don't think your name has '.' in it,
>right?

Right. It should've been Eun Taik Lee.

>Your email client turned all tabs to spaces and made the patch unable to
>be applied.

I think I pasted the patch on HTML mode and then switched to text mode.
Below patch should be okay.


From: "Eun Taik Lee" <eun.taik.lee@samsung.com>

There is a possible race condition when a device
is added while another device with the same parent
, with ref count of one, is deleted.

	CPU0				CPU1
device_add()			device_del()
get_device_parent()		put_device(parent);
kobj = kobject_get(k)		kobject_put()
				kref_put()
				refcount=0
refcount is 0
WARNS it was 0 but
return kobj			frees kobj
uses the freed parent kobj

The race condition exists because kref_put() and the
release function is not atomic.
Using kobject_get_unless_zero() instead of
kobject_get() in get_device_parent() will prevent
this race condition.

Signed-off-by: Eun Taik Lee <eun.taik.lee@samsung.com>
---
 drivers/base/core.c     |    2 +-
 include/linux/kobject.h |    2 ++
 lib/kobject.c           |    2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 07304a3..ec2f211 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -761,7 +761,7 @@ static struct kobject *get_device_parent(struct device *dev,
 		spin_lock(&dev->class->p->glue_dirs.list_lock);
 		list_for_each_entry(k, &dev->class->p->glue_dirs.list, entry)
 			if (k->parent == parent_kobj) {
-				kobj = kobject_get(k);
+				kobj = kobject_get_unless_zero(k);
 				break;
 			}
 		spin_unlock(&dev->class->p->glue_dirs.list_lock);
diff --git a/include/linux/kobject.h b/include/linux/kobject.h
index 2d61b90..3032744 100644
--- a/include/linux/kobject.h
+++ b/include/linux/kobject.h
@@ -107,6 +107,8 @@ extern int __must_check kobject_rename(struct kobject *, const char *new_name);
 extern int __must_check kobject_move(struct kobject *, struct kobject *);
 
 extern struct kobject *kobject_get(struct kobject *kobj);
+extern struct kobject * __must_check kobject_get_unless_zero(
+		struct kobject *kobj);
 extern void kobject_put(struct kobject *kobj);
 
 extern const void *kobject_namespace(struct kobject *kobj);
diff --git a/lib/kobject.c b/lib/kobject.c
index 03d4ab3..ff0ce681 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -581,7 +581,7 @@ struct kobject *kobject_get(struct kobject *kobj)
 	return kobj;
 }
 
-static struct kobject * __must_check kobject_get_unless_zero(struct kobject *kobj)
+struct kobject * __must_check kobject_get_unless_zero(struct kobject *kobj)
 {
 	if (!kref_get_unless_zero(&kobj->kref))
 		kobj = NULL;
-- 
1.7.9.5


thanks
Eun Taik Leeÿôèº{.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: Re: [PATCH] fix race condition between device_del and device_add

[gregkh]

On Mon, Mar 30, 2015 at 11:54:54AM +0000, EunTaik Lee wrote:
> 
> >
> >The bus that the device is on should prevent this, why isn't that
> >working for you?  What type of device/bus do you see this problem on?
> 
> The device is a firmware class device on a virtual bus.

What kernel code is causing this?  Again, the bus should be preventing
this from happening, is the virtual bus code incorrect somehow?

> >> Signed-off-by: eun.taik.lee 
> >
> >We need a "real" name here, I don't think your name has '.' in it,
> >right?
> 
> Right. It should've been Eun Taik Lee.
> 
> >Your email client turned all tabs to spaces and made the patch unable to
> >be applied.
> 
> I think I pasted the patch on HTML mode and then switched to text mode.
> Below patch should be okay.

I can't take a patch like this, please resend it properly.

thanks,

greg k-h