From e4a60d139060975eb956717e4f63ae348d4d8cc5 Mon Sep 17 00:00:00 2001 From: Yijing Wang Date: Fri, 7 Nov 2014 12:05:49 +0800 Subject: sysfs: driver core: Fix glue dir race condition by gdp_mutex 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 is a "classic" problem we have of a kref in a list that can be found while the last instance could be removed at the same time. This patch reuse gdp_mutex to fix this race condition. 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] [] warn_slowpath_common+0x7a/0xb0 <4>[ 3965.441615] [] warn_slowpath_null+0x15/0x20 <4>[ 3965.441618] [] kobject_get+0x33/0x40 <4>[ 3965.441624] [] get_device_parent.isra.11+0x135/0x1f0 <4>[ 3965.441627] [] device_add+0xd4/0x6d0 <4>[ 3965.441631] [] ? 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] [] sysfs_create_group+0xe/0x10 <4>[ 3965.686748] [] blk_trace_init_sysfs+0x14/0x20 <4>[ 3965.686753] [] blk_register_queue+0x3b/0x120 <4>[ 3965.686756] [] add_disk+0x1cc/0x490 .... ------------------------------------------------------- Signed-off-by: Yijing Wang Signed-off-by: Weng Meiling Cc: #3.4+ Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 14d162952c3b..842d04707de6 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -724,12 +724,12 @@ class_dir_create_and_add(struct class *class, struct kobject *parent_kobj) return &dir->kobj; } +static DEFINE_MUTEX(gdp_mutex); static struct kobject *get_device_parent(struct device *dev, struct device *parent) { if (dev->class) { - static DEFINE_MUTEX(gdp_mutex); struct kobject *kobj = NULL; struct kobject *parent_kobj; struct kobject *k; @@ -793,7 +793,9 @@ static void cleanup_glue_dir(struct device *dev, struct kobject *glue_dir) glue_dir->kset != &dev->class->p->glue_dirs) return; + mutex_lock(&gdp_mutex); kobject_put(glue_dir); + mutex_unlock(&gdp_mutex); } static void cleanup_device_parent(struct device *dev) -- cgit v1.2.3-58-ga151 From 9c6026994c2a18473c9ef9ed77e8cf8e344f4357 Mon Sep 17 00:00:00 2001 From: Aristeu Rozanski Date: Thu, 16 Oct 2014 11:49:49 -0400 Subject: tiny: reverse logic for DISABLE_DEV_COREDUMP It's desirable for allnconfig and tinyconfig targets to result in the least amount of code possible. DISABLE_DEV_COREDUMP exists as a way to switch off DEV_COREDUMP regardless if any drivers select WANT_DEV_COREDUMP. This patch renames the option to ENABLE_DEV_COREDUMP and setting it to 'n' (as in allnconfig or tinyconfig) will effectively disable device coredump. Cc: Josh Triplett Reviewed-by: Josh Triplett Signed-off-by: Aristeu Rozanski Reviewed-by: Johannes Berg Acked-by: Johannes Berg Signed-off-by: Greg Kroah-Hartman --- drivers/base/Kconfig | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index 61a33f4ba608..c29d1600e90b 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -171,20 +171,23 @@ config WANT_DEV_COREDUMP Drivers should "select" this option if they desire to use the device coredump mechanism. -config DISABLE_DEV_COREDUMP - bool "Disable device coredump" if EXPERT +config ENABLE_DEV_COREDUMP + bool "Enable device coredump" if EXPERT + default y help - Disable the device coredump mechanism despite drivers wanting to - use it; this allows for more sensitive systems or systems that - don't want to ever access the information to not have the code, - nor keep any data. + This option controls if the device coredump mechanism is available or + not; if disabled, the mechanism will be omitted even if drivers that + can use it are enabled. + Say 'N' for more sensitive systems or systems that don't want + to ever access the information to not have the code, nor keep any + data. - If unsure, say N. + If unsure, say Y. config DEV_COREDUMP bool default y if WANT_DEV_COREDUMP - depends on !DISABLE_DEV_COREDUMP + depends on ENABLE_DEV_COREDUMP config DEBUG_DRIVER bool "Driver Core verbose debug messages" -- cgit v1.2.3-58-ga151 From cd3d9ea142c9fd47351e79fe30c7ae2c86302cda Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 30 Oct 2014 10:00:35 +0100 Subject: tiny: rename ENABLE_DEV_COREDUMP to ALLOW_DEV_COREDUMP The ENABLE_DEV_COREDUMP option is misleading as it implies that it gets the framework enabled, this isn't true it just allows it to get enabled if a driver needs it. Rename it to ALLOW_DEV_COREDUMP to better capture its semantics. Signed-off-by: Johannes Berg Reviewed-by: Josh Triplett Acked-by: Aristeu Rozanski Signed-off-by: Greg Kroah-Hartman --- drivers/base/Kconfig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index c29d1600e90b..df04227d00cf 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -171,8 +171,8 @@ config WANT_DEV_COREDUMP Drivers should "select" this option if they desire to use the device coredump mechanism. -config ENABLE_DEV_COREDUMP - bool "Enable device coredump" if EXPERT +config ALLOW_DEV_COREDUMP + bool "Allow device coredump" if EXPERT default y help This option controls if the device coredump mechanism is available or @@ -187,7 +187,7 @@ config ENABLE_DEV_COREDUMP config DEV_COREDUMP bool default y if WANT_DEV_COREDUMP - depends on ENABLE_DEV_COREDUMP + depends on ALLOW_DEV_COREDUMP config DEBUG_DRIVER bool "Driver Core verbose debug messages" -- cgit v1.2.3-58-ga151