[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <m163tocw43.fsf_-_@frodo.ebiederm.org>
Date: Thu, 08 May 2008 14:41:00 -0700
From: ebiederm@...ssion.com (Eric W. Biederman)
To: Greg KH <gregkh@...e.de>, Andrew Morton <akpm@...ux-foundation.org>
Cc: Benjamin Thery <benjamin.thery@...l.net>,
linux-kernel@...r.kernel.org, Tejun Heo <htejun@...il.com>,
Al Viro <viro@....linux.org.uk>,
Daniel Lezcano <dlezcano@...ibm.com>,
"Serge E. Hallyn" <serue@...ibm.com>,
Pavel Emelyanov <xemul@...nvz.org>, netdev@...r.kernel.org
Subject: [PATCH] Fix kobject_rename and !CONFIG_SYSFS
When looking at kobject_rename I found two bugs with
that exist when sysfs support is disabled in the kernel.
kobject_rename does not change the name on the kobject when
sysfs support is not compiled in.
kobject_rename without locking attempts to check the
validity of a rename operation, which the kobject layer
simply does not have the infrastructure to do so.
This patch documents the previously unstated requirement of
kobject_rename that is the responsibility of the caller to
provide mutual exclusion and to be certain that the new_name
for the kobject is valid.
This patch modifies sysfs_rename_dir in !CONFIG_SYSFS case
to call kobject_set_name to actually change the kobject_name.
This patch removes the bogus and misleading check in kobject_rename
that attempts to see if a rename is valid. The check is bogus
because we do not have the proper locking. The check is misleading
because it looks like we can and do perform useful checking at the
kobject level that we don't.
The users in net/core/dev.c already have all of the necessary checks
in place and don't care. The other use in net/core/wireless.c
as originally implemented is incorrect because it performs no locking
and a simple patch has been sent that adds the necessary locking
and sanity checks there. Ensuring this patch will not have an
effect on users of kobject_rename or device_rename.
Signed-off-by: Eric W. Biederman <ebiederm@...ssion.com>
---
Documentation/kobject.txt | 4 ++++
drivers/base/core.c | 5 +++++
include/linux/sysfs.h | 2 +-
lib/kobject.c | 18 +++++-------------
4 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/Documentation/kobject.txt b/Documentation/kobject.txt
index bf3256e..79184b4 100644
--- a/Documentation/kobject.txt
+++ b/Documentation/kobject.txt
@@ -118,6 +118,10 @@ the name of the kobject, call kobject_rename():
int kobject_rename(struct kobject *kobj, const char *new_name);
+Note kobject_rename does perform any locking or have a solid notion of
+what names are valid so the provide must provide their own sanity checking
+and serialization.
+
There is a function called kobject_set_name() but that is legacy cruft and
is being removed. If your code needs to call this function, it is
incorrect and needs to be fixed.
diff --git a/drivers/base/core.c b/drivers/base/core.c
index be288b5..ad68f4c 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1171,6 +1171,11 @@ EXPORT_SYMBOL_GPL(device_destroy);
* device_rename - renames a device
* @dev: the pointer to the struct device to be renamed
* @new_name: the new name of the device
+ *
+ * It is the responsibility of the caller to provide mutual
+ * exclusion between two different calls of device_rename
+ * on the same device to ensure that new_name is valid and
+ * won't conflict with other devices.
*/
int device_rename(struct device *dev, char *new_name)
{
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 7858eac..db66fa5 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -137,7 +137,7 @@ static inline void sysfs_remove_dir(struct kobject *kobj)
static inline int sysfs_rename_dir(struct kobject *kobj, const char *new_name)
{
- return 0;
+ return kobject_set_name(kobj, "%s", new_name);
}
static inline int sysfs_move_dir(struct kobject *kobj,
diff --git a/lib/kobject.c b/lib/kobject.c
index 718e510..c7fb092 100644
--- a/lib/kobject.c
+++ b/lib/kobject.c
@@ -383,6 +383,11 @@ EXPORT_SYMBOL_GPL(kobject_init_and_add);
* kobject_rename - change the name of an object
* @kobj: object in question.
* @new_name: object's new name
+ *
+ * It is the responsibility of the caller to provide mutual
+ * exclusion between two different calls of kobject_rename
+ * on the same kobject and to ensure that new_name is valid and
+ * won't conflict with other kobjects.
*/
int kobject_rename(struct kobject *kobj, const char *new_name)
{
@@ -397,19 +402,6 @@ int kobject_rename(struct kobject *kobj, const char *new_name)
if (!kobj->parent)
return -EINVAL;
- /* see if this name is already in use */
- if (kobj->kset) {
- struct kobject *temp_kobj;
- temp_kobj = kset_find_obj(kobj->kset, new_name);
- if (temp_kobj) {
- printk(KERN_WARNING "kobject '%s' cannot be renamed "
- "to '%s' as '%s' is already in existence.\n",
- kobject_name(kobj), new_name, new_name);
- kobject_put(temp_kobj);
- return -EINVAL;
- }
- }
-
devpath = kobject_get_path(kobj, GFP_KERNEL);
if (!devpath) {
error = -ENOMEM;
--
1.5.3.rc6.17.g1911
--
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/
Powered by blists - more mailing lists