linux-kernel - Re: Re: [PATCH] fix race condition between device_del and device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 30 Mar 2015 11:54:55 +0000 (GMT)
From:	EunTaik Lee <eun.taik.lee@...sung.com>
To:	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: Re: [PATCH] fix race condition between device_del and device_add


>
>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@...sung.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@...sung.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