linux-kernel - [PATCH] sysfs: driver core: Fix glue dir race condition

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:	Thu, 6 Nov 2014 16:16:38 +0800
From:	Yijing Wang <wangyijing@...wei.com>
To:	<gregkh@...uxfoundation.org>
CC:	<tj@...nel.org>, <lizefan@...wei.com>,
	<linux-kernel@...r.kernel.org>,
	Yijing Wang <wangyijing@...wei.com>,
	Weng Meiling <wengmeiling.weng@...wei.com>,
	<stable@...r.kernel.org>
Subject: [PATCH] sysfs: driver core: Fix glue dir race condition

There is a race condition when removing glue directory.
It can be reproduced in following test:

path 1: Add first child device
device_add()
	get_device_parent()
		/*find parent from glue_dirs.list*/
		list_for_each_entry(k, &dev->class->p->glue_dirs.list, entry)
			if (k->parent == parent_kobj) {
				kobj = kobject_get(k);
				break;
			}
		....
		class_dir_create_and_add()

path2: Remove last child device under glue dir
device_del()
	cleanup_device_parent()
		cleanup_glue_dir()
			kobject_put(glue_dir);

If path2 has been called cleanup_glue_dir(), but not
call kobject_put(glue_dir), the glue dir is still
in parent's kset list. Meanwhile, path1 find the glue
dir from the glue_dirs.list. Path2 may release glue dir
before path1 call kobject_get(). So kernel will report
the warning and bug_on.

This fix keep glue dir around once it created suggested
by Tejun Heo.

The following calltrace is captured in kernel 3.4, but
the latest kernel still has this bug.

-----------------------------------------------------
<4>[ 3965.441471] WARNING: at ...include/linux/kref.h:41 kobject_get+0x33/0x40()
<4>[ 3965.441474] Hardware name: Romley
<4>[ 3965.441475] Modules linked in: isd_iop(O) isd_xda(O)...
...
<4>[ 3965.441605] Call Trace:
<4>[ 3965.441611]  [<ffffffff8103717a>] warn_slowpath_common+0x7a/0xb0
<4>[ 3965.441615]  [<ffffffff810371c5>] warn_slowpath_null+0x15/0x20
<4>[ 3965.441618]  [<ffffffff81215963>] kobject_get+0x33/0x40
<4>[ 3965.441624]  [<ffffffff812d1e45>] get_device_parent.isra.11+0x135/0x1f0
<4>[ 3965.441627]  [<ffffffff812d22d4>] device_add+0xd4/0x6d0
<4>[ 3965.441631]  [<ffffffff812d0dbc>] ? dev_set_name+0x3c/0x40
....
<2>[ 3965.441912] kernel BUG at ..../fs/sysfs/group.c:65!
<4>[ 3965.441915] invalid opcode: 0000 [#1] SMP
...
<4>[ 3965.686743]  [<ffffffff811a677e>] sysfs_create_group+0xe/0x10
<4>[ 3965.686748]  [<ffffffff810cfb04>] blk_trace_init_sysfs+0x14/0x20
<4>[ 3965.686753]  [<ffffffff811fcabb>] blk_register_queue+0x3b/0x120
<4>[ 3965.686756]  [<ffffffff812030bc>] add_disk+0x1cc/0x490
....
-------------------------------------------------------

Signed-off-by: Yijing Wang <wangyijing@...wei.com>
Signed-off-by: Weng Meiling <wengmeiling.weng@...wei.com>
Cc: <stable@...r.kernel.org> #3.4+
---
 drivers/base/core.c |   21 +--------------------
 1 files changed, 1 insertions(+), 20 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 14d1629..5b7a3e9 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 = k;
 				break;
 			}
 		spin_unlock(&dev->class->p->glue_dirs.list_lock);
@@ -786,21 +786,6 @@ static struct kobject *get_device_parent(struct device *dev,
 	return NULL;
 }
 
-static void cleanup_glue_dir(struct device *dev, struct kobject *glue_dir)
-{
-	/* see if we live in a "glue" directory */
-	if (!glue_dir || !dev->class ||
-	    glue_dir->kset != &dev->class->p->glue_dirs)
-		return;
-
-	kobject_put(glue_dir);
-}
-
-static void cleanup_device_parent(struct device *dev)
-{
-	cleanup_glue_dir(dev, dev->kobj.parent);
-}
-
 static int device_add_class_symlinks(struct device *dev)
 {
 	int error;
@@ -1094,7 +1079,6 @@ done:
 	kobject_uevent(&dev->kobj, KOBJ_REMOVE);
 	kobject_del(&dev->kobj);
  Error:
-	cleanup_device_parent(dev);
 	if (parent)
 		put_device(parent);
 name_error:
@@ -1215,7 +1199,6 @@ void device_del(struct device *dev)
 		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
 					     BUS_NOTIFY_REMOVED_DEVICE, dev);
 	kobject_uevent(&dev->kobj, KOBJ_REMOVE);
-	cleanup_device_parent(dev);
 	kobject_del(&dev->kobj);
 	put_device(parent);
 }
@@ -1873,7 +1856,6 @@ int device_move(struct device *dev, struct device *new_parent,
 		 __func__, new_parent ? dev_name(new_parent) : "<NULL>");
 	error = kobject_move(&dev->kobj, new_parent_kobj);
 	if (error) {
-		cleanup_glue_dir(dev, new_parent_kobj);
 		put_device(new_parent);
 		goto out;
 	}
@@ -1902,7 +1884,6 @@ int device_move(struct device *dev, struct device *new_parent,
 					set_dev_node(dev, dev_to_node(old_parent));
 				}
 			}
-			cleanup_glue_dir(dev, new_parent_kobj);
 			put_device(new_parent);
 			goto out;
 		}
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/