linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Greg KH <greg@kroah.com>
Cc: Benjamin Thery <benjamin.thery@bull.net>,
	Randy Dunlap <randy.dunlap@oracle.com>, Greg KH <gregkh@suse.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-kernel@vger.kernel.org, Tejun Heo <htejun@gmail.com>,
	Al Viro <viro@ftp.linux.org.uk>,
	Daniel Lezcano <dlezcano@fr.ibm.com>,
	"Serge E. Hallyn" <serue@us.ibm.com>,
	Pavel Emelyanov <xemul@openvz.org>,
	netdev@vger.kernel.org
Subject: [PATCH] Fix kobject_rename and !CONFIG_SYSFS v3
Date: Tue, 13 May 2008 13:45:16 -0700	[thread overview]
Message-ID: <m1od7a9bmr.fsf_-_@frodo.ebiederm.org> (raw)
In-Reply-To: <20080513201602.GA31578@kroah.com> (Greg KH's message of "Tue, 13 May 2008 13:16:02 -0700")


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.

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 checking at the kobject
level that we don't.

Changelog:
v3:	Documentation typo fixes

v2:	Added a declaration of kboject_set_name to sysfs.h
	so the code actually compiles with !CONFIG_SYSFS.

	Unscrambling the header dependencies so everything looks
	beautiful is a project for another day.

Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
 Documentation/kobject.txt |    4 ++++
 drivers/base/core.c       |    5 +++++
 include/linux/sysfs.h     |    4 +++-
 lib/kobject.c             |   18 +++++-------------
 4 files changed, 17 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..6e61033 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -20,6 +20,8 @@
 struct kobject;
 struct module;
 
+extern int kobject_set_name(struct kobject *kobj, const char *name, ...)
+			    __attribute__((format(printf, 2, 3)));
 /* FIXME
  * The *owner field is no longer used, but leave around
  * until the tree gets cleaned up fully.
@@ -137,7 +139,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


  reply	other threads:[~2008-05-13 20:51 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-06 17:30 [RESEND][PATCH 00/11] sysfs tagged directories Benjamin Thery
2008-05-06 17:30 ` [PATCH 01/11] sysfs: Support for preventing unmounts Benjamin Thery
2008-05-06 17:30 ` [PATCH 02/11] sysfs: sysfs_get_dentry add a sb parameter Benjamin Thery
2008-05-06 17:31 ` [PATCH 03/11] sysfs: Implement __sysfs_get_dentry Benjamin Thery
2008-05-06 17:31 ` [PATCH 04/11] sysfs: Rename Support multiple superblocks Benjamin Thery
2008-05-06 17:31 ` [PATCH 05/11] sysfs: sysfs_chmod_file handle " Benjamin Thery
2008-05-06 17:31 ` [PATCH 06/11] sysfs: Implement sysfs tagged directory support Benjamin Thery
2008-05-06 17:31 ` [PATCH 07/11] sysfs: Implement sysfs_delete_link and sysfs_rename_link Benjamin Thery
2008-05-06 17:31 ` [PATCH 08/11] driver core: Implement tagged directory support for device classes Benjamin Thery
2008-05-06 17:32 ` [PATCH 09/11] netns: Enable tagging for net_class directories in sysfs Benjamin Thery
2008-05-06 17:32 ` [PATCH 10/11] avoid kobject name conflict with different namespaces Benjamin Thery
2008-05-07 18:49   ` Eric W. Biederman
2008-05-07 19:08     ` Greg KH
2008-05-07 20:54       ` Eric W. Biederman
2008-05-08  8:28         ` Cornelia Huck
2008-05-08 19:28           ` Eric W. Biederman
2008-05-09  5:35             ` Cornelia Huck
2008-05-09 18:16               ` Eric W. Biederman
2008-05-08 19:25       ` Eric W. Biederman
2008-05-08 21:30       ` [PATCH] wireless: Add missing locking to cfg80211_dev_rename Eric W. Biederman
2008-05-08 22:12         ` Serge E. Hallyn
2008-05-08 22:18         ` Johannes Berg
2008-05-08 21:41       ` [PATCH] Fix kobject_rename and !CONFIG_SYSFS Eric W. Biederman
2008-05-12 22:02         ` kobject: " Greg KH
2008-05-13  7:00           ` Eric W. Biederman
2008-05-13 14:25             ` Benjamin Thery
2008-05-13 16:44               ` Greg KH
2008-05-13 17:55                 ` [PATCH] Fix kobject_rename and !CONFIG_SYSFS v2 Eric W. Biederman
2008-05-13 18:23                   ` Randy.Dunlap
2008-05-13 20:43                     ` Eric W. Biederman
2008-05-13 20:16                   ` Greg KH
2008-05-13 20:45                     ` Eric W. Biederman [this message]
2008-05-13 21:18                       ` [PATCH] Fix kobject_rename and !CONFIG_SYSFS v3 Randy Dunlap
2008-05-14  4:39                         ` [PATCH] Fix kobject_rename and !CONFIG_SYSFS v4 Eric W. Biederman
2008-05-14  5:03                           ` Andrew Morton
2008-05-14  9:01                             ` Eric W. Biederman
2008-05-14  9:20                               ` Andrew Morton
2008-05-14  9:51                                 ` Benjamin Thery
2008-05-14  9:56                                   ` Andrew Morton
2008-05-13 19:33                 ` kobject: Fix kobject_rename and !CONFIG_SYSFS Benjamin Thery 
2008-05-13 20:42                   ` Eric W. Biederman
2008-05-06 17:32 ` [PATCH 11/11] sysfs: user namespaces: add ns to user_struct Benjamin Thery
2008-05-06 19:03   ` Serge E. Hallyn
2008-05-06 17:53 ` [RESEND][PATCH 00/11] sysfs tagged directories Greg KH
2008-05-06 18:41   ` Benjamin Thery
2008-05-07 13:19   ` Daniel Lezcano
2008-05-07 13:47     ` Benjamin Thery
2008-05-14 15:07     ` Benjamin Thery

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m1od7a9bmr.fsf_-_@frodo.ebiederm.org \
    --to=ebiederm@xmission.com \
    --cc=akpm@linux-foundation.org \
    --cc=benjamin.thery@bull.net \
    --cc=dlezcano@fr.ibm.com \
    --cc=greg@kroah.com \
    --cc=gregkh@suse.de \
    --cc=htejun@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=randy.dunlap@oracle.com \
    --cc=serue@us.ibm.com \
    --cc=viro@ftp.linux.org.uk \
    --cc=xemul@openvz.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).