linux-kernel - [PATCH 2/2] drivers: core: Remove glue dirs from sysfs earlier

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]

Message-ID: <ac64e45950c8e3df26fd50e7b251d5895c1d2201.camel@kernel.crashing.org>
Date:   Fri, 29 Jun 2018 12:21:58 +1000
From:   Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Joel Stanley <joel@....id.au>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: [PATCH 2/2] drivers: core: Remove glue dirs from sysfs earlier

For devices with a class, we create a "glue" directory between
the parent device and the new device with the class name.

This directory is never "explicitely" removed when empty however,
this is left to the implicit sysfs removal done by kobjects when
they are released on the last kobject_put().

This is problematic because as long as it's not been removed from
sysfs, it is still present in the class kset and in sysfs directory
structure.

The presence in the class kset exposes a use after free bug fixed
by the previous patch, but the presence in sysfs means that until
the kobject is released, which can take a while (especially with
kobject debugging), any attempt at re-creating such as binding a
new device for that class/parent pair, will result in a sysfs
duplicate file name error.

This fixes it by instead doing an explicit kobject_del() when
the glue dir is empty, by keeping track of the number of
child devices of the gluedir.

This is made easy by the fact that all glue dir operations are
done with a global mutex, and there's already a function
(cleanup_glue_dir) called in all the right places taking that
mutex that can be enhanced for this.

Signed-off-by: Benjamin Herrenschmidt <benh@...nel.crashing.org>
---

(Adding lkml, I just realized I completely forgot to CC it in
the first place on this whole conversation, blame the 1am debugging
session)

 drivers/base/core.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index e9eff2099896..66e15daa9980 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1436,6 +1436,13 @@ struct kobject *virtual_device_parent(struct device *dev)
 struct class_dir {
 	struct kobject kobj;
 	struct class *class;
+
+	/*
+	 * This counts the number of children of the gluedir in
+	 * order to "cleanly" kobject_del() it when it becomes
+	 * empty. It is protected by the gdb_mutex
+	 */
+	unsigned int child_count;
 };

 #define to_class_dir(obj) container_of(obj, struct class_dir, kobj)
@@ -1470,6 +1477,7 @@ class_dir_create_and_add(struct class *class, struct kobject *parent_kobj)
 		return NULL;

 	dir->class = class;
+	dir->child_count = 1;
 	kobject_init(&dir->kobj, &class_dir_ktype);

 	dir->kobj.kset = &class->p->glue_dirs;
@@ -1526,6 +1534,8 @@ static struct kobject *get_device_parent(struct device *dev,
 		}
 		spin_unlock(&dev->class->p->glue_dirs.list_lock);
 		if (kobj) {
+			struct class_dir *class_dir = to_class_dir(kobj);
+			class_dir->child_count++;
 			mutex_unlock(&gdp_mutex);
 			return kobj;
 		}
@@ -1567,11 +1577,18 @@ static inline struct kobject *get_glue_dir(struct device *dev)
  */
 static void cleanup_glue_dir(struct device *dev, struct kobject *glue_dir)
 {
+	struct class_dir *class_dir = to_class_dir(glue_dir);
+
 	/* see if we live in a "glue" directory */
 	if (!live_in_glue_dir(glue_dir, dev))
 		return;

 	mutex_lock(&gdp_mutex);
+	if (WARN_ON(class_dir->child_count == 0))
+		goto bail;
+	if (--class_dir->child_count == 0)
+		kobject_del(glue_dir);
+ bail:
 	kobject_put(glue_dir);
 	mutex_unlock(&gdp_mutex);
 }