From f38506c49dab2751567423865941f32f2ea61c45 Mon Sep 17 00:00:00 2001 From: Stefan Richter Date: Wed, 14 Oct 2009 20:47:32 +0200 Subject: sysfs: mark a locally-only used function static Signed-off-by: Stefan Richter Acked-by: David P. Quigley Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/inode.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c index e28cecf179f5..e1443518398e 100644 --- a/fs/sysfs/inode.c +++ b/fs/sysfs/inode.c @@ -46,7 +46,7 @@ int __init sysfs_inode_init(void) return bdi_init(&sysfs_backing_dev_info); } -struct sysfs_inode_attrs *sysfs_init_inode_attrs(struct sysfs_dirent *sd) +static struct sysfs_inode_attrs *sysfs_init_inode_attrs(struct sysfs_dirent *sd) { struct sysfs_inode_attrs *attrs; struct iattr *iattrs; @@ -64,6 +64,7 @@ struct sysfs_inode_attrs *sysfs_init_inode_attrs(struct sysfs_dirent *sd) return attrs; } + int sysfs_setattr(struct dentry * dentry, struct iattr * iattr) { struct inode * inode = dentry->d_inode; -- cgit v1.2.3 From d3a3b0adad0865c12e39b712ca89efbd0a3a0dbc Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Tue, 17 Nov 2009 14:40:26 -0800 Subject: debugfs: fix create mutex racy fops and private data Setting fops and private data outside of the mutex at debugfs file creation introduces a race where the files can be opened with the wrong file operations and private data. It is easy to trigger with a process waiting on file creation notification. Signed-off-by: Mathieu Desnoyers Signed-off-by: Andrew Morton Cc: stable Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/inode.c | 55 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 23 deletions(-) (limited to 'fs') diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index 0d23b52dd22c..b486169f42bf 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -32,7 +32,9 @@ static struct vfsmount *debugfs_mount; static int debugfs_mount_count; static bool debugfs_registered; -static struct inode *debugfs_get_inode(struct super_block *sb, int mode, dev_t dev) +static struct inode *debugfs_get_inode(struct super_block *sb, int mode, dev_t dev, + void *data, const struct file_operations *fops) + { struct inode *inode = new_inode(sb); @@ -44,14 +46,18 @@ static struct inode *debugfs_get_inode(struct super_block *sb, int mode, dev_t d init_special_inode(inode, mode, dev); break; case S_IFREG: - inode->i_fop = &debugfs_file_operations; + inode->i_fop = fops ? fops : &debugfs_file_operations; + inode->i_private = data; break; case S_IFLNK: inode->i_op = &debugfs_link_operations; + inode->i_fop = fops; + inode->i_private = data; break; case S_IFDIR: inode->i_op = &simple_dir_inode_operations; - inode->i_fop = &simple_dir_operations; + inode->i_fop = fops ? fops : &simple_dir_operations; + inode->i_private = data; /* directory inodes start off with i_nlink == 2 * (for "." entry) */ @@ -64,7 +70,8 @@ static struct inode *debugfs_get_inode(struct super_block *sb, int mode, dev_t d /* SMP-safe */ static int debugfs_mknod(struct inode *dir, struct dentry *dentry, - int mode, dev_t dev) + int mode, dev_t dev, void *data, + const struct file_operations *fops) { struct inode *inode; int error = -EPERM; @@ -72,7 +79,7 @@ static int debugfs_mknod(struct inode *dir, struct dentry *dentry, if (dentry->d_inode) return -EEXIST; - inode = debugfs_get_inode(dir->i_sb, mode, dev); + inode = debugfs_get_inode(dir->i_sb, mode, dev, data, fops); if (inode) { d_instantiate(dentry, inode); dget(dentry); @@ -81,12 +88,13 @@ static int debugfs_mknod(struct inode *dir, struct dentry *dentry, return error; } -static int debugfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) +static int debugfs_mkdir(struct inode *dir, struct dentry *dentry, int mode, + void *data, const struct file_operations *fops) { int res; mode = (mode & (S_IRWXUGO | S_ISVTX)) | S_IFDIR; - res = debugfs_mknod(dir, dentry, mode, 0); + res = debugfs_mknod(dir, dentry, mode, 0, data, fops); if (!res) { inc_nlink(dir); fsnotify_mkdir(dir, dentry); @@ -94,18 +102,20 @@ static int debugfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) return res; } -static int debugfs_link(struct inode *dir, struct dentry *dentry, int mode) +static int debugfs_link(struct inode *dir, struct dentry *dentry, int mode, + void *data, const struct file_operations *fops) { mode = (mode & S_IALLUGO) | S_IFLNK; - return debugfs_mknod(dir, dentry, mode, 0); + return debugfs_mknod(dir, dentry, mode, 0, data, fops); } -static int debugfs_create(struct inode *dir, struct dentry *dentry, int mode) +static int debugfs_create(struct inode *dir, struct dentry *dentry, int mode, + void *data, const struct file_operations *fops) { int res; mode = (mode & S_IALLUGO) | S_IFREG; - res = debugfs_mknod(dir, dentry, mode, 0); + res = debugfs_mknod(dir, dentry, mode, 0, data, fops); if (!res) fsnotify_create(dir, dentry); return res; @@ -139,7 +149,9 @@ static struct file_system_type debug_fs_type = { static int debugfs_create_by_name(const char *name, mode_t mode, struct dentry *parent, - struct dentry **dentry) + struct dentry **dentry, + void *data, + const struct file_operations *fops) { int error = 0; @@ -164,13 +176,16 @@ static int debugfs_create_by_name(const char *name, mode_t mode, if (!IS_ERR(*dentry)) { switch (mode & S_IFMT) { case S_IFDIR: - error = debugfs_mkdir(parent->d_inode, *dentry, mode); + error = debugfs_mkdir(parent->d_inode, *dentry, mode, + data, fops); break; case S_IFLNK: - error = debugfs_link(parent->d_inode, *dentry, mode); + error = debugfs_link(parent->d_inode, *dentry, mode, + data, fops); break; default: - error = debugfs_create(parent->d_inode, *dentry, mode); + error = debugfs_create(parent->d_inode, *dentry, mode, + data, fops); break; } dput(*dentry); @@ -221,19 +236,13 @@ struct dentry *debugfs_create_file(const char *name, mode_t mode, if (error) goto exit; - error = debugfs_create_by_name(name, mode, parent, &dentry); + error = debugfs_create_by_name(name, mode, parent, &dentry, + data, fops); if (error) { dentry = NULL; simple_release_fs(&debugfs_mount, &debugfs_mount_count); goto exit; } - - if (dentry->d_inode) { - if (data) - dentry->d_inode->i_private = data; - if (fops) - dentry->d_inode->i_fop = fops; - } exit: return dentry; } -- cgit v1.2.3 From f44d3e7857e5d21413fc9a2b61948190c73705e7 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sat, 7 Nov 2009 23:26:59 -0800 Subject: sysfs: Update sysfs_setxattr so it updates secdata under the sysfs_mutex The sysfs_mutex is required to ensure updates are and will remain atomic with respect to other inode iattr updates, that do not happen through the filesystem. Acked-by: Serge Hallyn Acked-by: Tejun Heo Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/inode.c | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c index e1443518398e..000bd9865004 100644 --- a/fs/sysfs/inode.c +++ b/fs/sysfs/inode.c @@ -123,23 +123,39 @@ int sysfs_setattr(struct dentry * dentry, struct iattr * iattr) return error; } +static int sysfs_sd_setsecdata(struct sysfs_dirent *sd, void **secdata, u32 *secdata_len) +{ + struct sysfs_inode_attrs *iattrs; + void *old_secdata; + size_t old_secdata_len; + + iattrs = sd->s_iattr; + if (!iattrs) + iattrs = sysfs_init_inode_attrs(sd); + if (!iattrs) + return -ENOMEM; + + old_secdata = iattrs->ia_secdata; + old_secdata_len = iattrs->ia_secdata_len; + + iattrs->ia_secdata = *secdata; + iattrs->ia_secdata_len = *secdata_len; + + *secdata = old_secdata; + *secdata_len = old_secdata_len; + return 0; +} + int sysfs_setxattr(struct dentry *dentry, const char *name, const void *value, size_t size, int flags) { struct sysfs_dirent *sd = dentry->d_fsdata; - struct sysfs_inode_attrs *iattrs; void *secdata; int error; u32 secdata_len = 0; if (!sd) return -EINVAL; - if (!sd->s_iattr) - sd->s_iattr = sysfs_init_inode_attrs(sd); - if (!sd->s_iattr) - return -ENOMEM; - - iattrs = sd->s_iattr; if (!strncmp(name, XATTR_SECURITY_PREFIX, XATTR_SECURITY_PREFIX_LEN)) { const char *suffix = name + XATTR_SECURITY_PREFIX_LEN; @@ -151,12 +167,13 @@ int sysfs_setxattr(struct dentry *dentry, const char *name, const void *value, &secdata, &secdata_len); if (error) goto out; - if (iattrs->ia_secdata) - security_release_secctx(iattrs->ia_secdata, - iattrs->ia_secdata_len); - iattrs->ia_secdata = secdata; - iattrs->ia_secdata_len = secdata_len; + mutex_lock(&sysfs_mutex); + error = sysfs_sd_setsecdata(sd, &secdata, &secdata_len); + mutex_unlock(&sysfs_mutex); + + if (secdata) + security_release_secctx(secdata, secdata_len); } else return -EINVAL; out: -- cgit v1.2.3 From 28a027cfc0d527fcc31bfeac1d94d572c68847d1 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sat, 7 Nov 2009 23:27:00 -0800 Subject: sysfs: Rename sysfs_d_iput to sysfs_dentry_iput Using dentry instead of d in the function name is what several other filesystems are doing and it seems to be a more readable convention. Acked-by: Tejun Heo Acked-by: Serge Hallyn Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index e0201837d244..01a9a64ecd4e 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -298,7 +298,7 @@ void release_sysfs_dirent(struct sysfs_dirent * sd) goto repeat; } -static void sysfs_d_iput(struct dentry * dentry, struct inode * inode) +static void sysfs_dentry_iput(struct dentry *dentry, struct inode *inode) { struct sysfs_dirent * sd = dentry->d_fsdata; @@ -307,7 +307,7 @@ static void sysfs_d_iput(struct dentry * dentry, struct inode * inode) } static const struct dentry_operations sysfs_dentry_ops = { - .d_iput = sysfs_d_iput, + .d_iput = sysfs_dentry_iput, }; struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type) -- cgit v1.2.3 From e8f077c8831528e2ec1ea6c8ba090e405fdcd0b7 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sat, 7 Nov 2009 23:27:01 -0800 Subject: sysfs: Use dentry_ops instead of directly playing with the dcache Calling d_drop unconditionally when a sysfs_dirent is deleted has the potential to leak mounts, so instead implement dentry delete and revalidate operations that cause sysfs dentries to be removed at the appropriate time. Acked-by: Tejun Heo Acked-by: Serge Hallyn Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 73 ++++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 46 insertions(+), 27 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index 01a9a64ecd4e..9ee130be03be 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -298,6 +298,46 @@ void release_sysfs_dirent(struct sysfs_dirent * sd) goto repeat; } +static int sysfs_dentry_delete(struct dentry *dentry) +{ + struct sysfs_dirent *sd = dentry->d_fsdata; + return !!(sd->s_flags & SYSFS_FLAG_REMOVED); +} + +static int sysfs_dentry_revalidate(struct dentry *dentry, struct nameidata *nd) +{ + struct sysfs_dirent *sd = dentry->d_fsdata; + int is_dir; + + mutex_lock(&sysfs_mutex); + + /* The sysfs dirent has been deleted */ + if (sd->s_flags & SYSFS_FLAG_REMOVED) + goto out_bad; + + mutex_unlock(&sysfs_mutex); +out_valid: + return 1; +out_bad: + /* Remove the dentry from the dcache hashes. + * If this is a deleted dentry we use d_drop instead of d_delete + * so sysfs doesn't need to cope with negative dentries. + */ + is_dir = (sysfs_type(sd) == SYSFS_DIR); + mutex_unlock(&sysfs_mutex); + if (is_dir) { + /* If we have submounts we must allow the vfs caches + * to lie about the state of the filesystem to prevent + * leaks and other nasty things. + */ + if (have_submounts(dentry)) + goto out_valid; + shrink_dcache_parent(dentry); + } + d_drop(dentry); + return 0; +} + static void sysfs_dentry_iput(struct dentry *dentry, struct inode *inode) { struct sysfs_dirent * sd = dentry->d_fsdata; @@ -307,6 +347,8 @@ static void sysfs_dentry_iput(struct dentry *dentry, struct inode *inode) } static const struct dentry_operations sysfs_dentry_ops = { + .d_revalidate = sysfs_dentry_revalidate, + .d_delete = sysfs_dentry_delete, .d_iput = sysfs_dentry_iput, }; @@ -527,44 +569,21 @@ void sysfs_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd) } /** - * sysfs_drop_dentry - drop dentry for the specified sysfs_dirent + * sysfs_dec_nlink - Decrement link count for the specified sysfs_dirent * @sd: target sysfs_dirent * - * Drop dentry for @sd. @sd must have been unlinked from its + * Decrement nlink for @sd. @sd must have been unlinked from its * parent on entry to this function such that it can't be looked * up anymore. */ -static void sysfs_drop_dentry(struct sysfs_dirent *sd) +static void sysfs_dec_nlink(struct sysfs_dirent *sd) { struct inode *inode; - struct dentry *dentry; inode = ilookup(sysfs_sb, sd->s_ino); if (!inode) return; - /* Drop any existing dentries associated with sd. - * - * For the dentry to be properly freed we need to grab a - * reference to the dentry under the dcache lock, unhash it, - * and then put it. The playing with the dentry count allows - * dput to immediately free the dentry if it is not in use. - */ -repeat: - spin_lock(&dcache_lock); - list_for_each_entry(dentry, &inode->i_dentry, d_alias) { - if (d_unhashed(dentry)) - continue; - dget_locked(dentry); - spin_lock(&dentry->d_lock); - __d_drop(dentry); - spin_unlock(&dentry->d_lock); - spin_unlock(&dcache_lock); - dput(dentry); - goto repeat; - } - spin_unlock(&dcache_lock); - /* adjust nlink and update timestamp */ mutex_lock(&inode->i_mutex); @@ -611,7 +630,7 @@ void sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt) acxt->removed = sd->s_sibling; sd->s_sibling = NULL; - sysfs_drop_dentry(sd); + sysfs_dec_nlink(sd); sysfs_deactivate(sd); unmap_bin_file(sd); sysfs_put(sd); -- cgit v1.2.3 From 4c6974f51a981d14f13e36049d6307d3bcda550e Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sat, 7 Nov 2009 23:27:02 -0800 Subject: sysfs: Simplify sysfs_chmod_file semantics Currently every caller of sysfs_chmod_file happens at either file creation time to set a non-default mode or in response to a specific user requested space change in policy. Making timestamps of when the chmod happens and notification of a file changing mode uninteresting. Remove the unnecessary time stamp and filesystem change notification, and removes the last of the explicit inotify and donitfy support from sysfs. Acked-by: Tejun Heo Acked-by: Serge Hallyn Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/file.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index f5ea4680f15f..faa1a803caa9 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -604,17 +604,9 @@ int sysfs_chmod_file(struct kobject *kobj, struct attribute *attr, mode_t mode) mutex_lock(&inode->i_mutex); newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO); - newattrs.ia_valid = ATTR_MODE | ATTR_CTIME; - newattrs.ia_ctime = current_fs_time(inode->i_sb); + newattrs.ia_valid = ATTR_MODE; rc = sysfs_setattr(victim, &newattrs); - if (rc == 0) { - fsnotify_change(victim, newattrs.ia_valid); - mutex_lock(&sysfs_mutex); - victim_sd->s_mode = newattrs.ia_mode; - mutex_unlock(&sysfs_mutex); - } - mutex_unlock(&inode->i_mutex); out: dput(victim); -- cgit v1.2.3 From 4be3df28beec5605c77a18aa2a4f987b5648f9ce Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sat, 7 Nov 2009 23:27:03 -0800 Subject: sysfs: Simplify iattr time assignments The granularity of sysfs time when we keep it is 1 ns. Which when passed to timestamp_trunc results in a nop. So remove the unnecessary function call making sysfs_setattr slightly easier to read. Acked-by: Tejun Heo Acked-by: Serge Hallyn Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/inode.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c index 000bd9865004..86fb230de432 100644 --- a/fs/sysfs/inode.c +++ b/fs/sysfs/inode.c @@ -104,14 +104,11 @@ int sysfs_setattr(struct dentry * dentry, struct iattr * iattr) if (ia_valid & ATTR_GID) iattrs->ia_gid = iattr->ia_gid; if (ia_valid & ATTR_ATIME) - iattrs->ia_atime = timespec_trunc(iattr->ia_atime, - inode->i_sb->s_time_gran); + iattrs->ia_atime = iattr->ia_atime; if (ia_valid & ATTR_MTIME) - iattrs->ia_mtime = timespec_trunc(iattr->ia_mtime, - inode->i_sb->s_time_gran); + iattrs->ia_mtime = iattr->ia_mtime; if (ia_valid & ATTR_CTIME) - iattrs->ia_ctime = timespec_trunc(iattr->ia_ctime, - inode->i_sb->s_time_gran); + iattrs->ia_ctime = iattr->ia_ctime; if (ia_valid & ATTR_MODE) { umode_t mode = iattr->ia_mode; -- cgit v1.2.3 From 35df63c46c8605d00140ec4833d2104e2e9f1acc Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 20 Nov 2009 16:08:50 -0800 Subject: sysfs: Fix locking and factor out sysfs_sd_setattr Cleanly separate the work that is specific to setting the attributes of a sysfs_dirent from what is needed to update the attributes of a vfs inode. Additionally grab the sysfs_mutex to keep any nasties from surprising us when updating the sysfs_dirent. Acked-by: Tejun Heo Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/inode.c | 51 +++++++++++++++++++++++++++++++-------------------- fs/sysfs/sysfs.h | 1 + 2 files changed, 32 insertions(+), 20 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c index 86fb230de432..76e977c300f7 100644 --- a/fs/sysfs/inode.c +++ b/fs/sysfs/inode.c @@ -65,30 +65,14 @@ static struct sysfs_inode_attrs *sysfs_init_inode_attrs(struct sysfs_dirent *sd) return attrs; } -int sysfs_setattr(struct dentry * dentry, struct iattr * iattr) +int sysfs_sd_setattr(struct sysfs_dirent *sd, struct iattr * iattr) { - struct inode * inode = dentry->d_inode; - struct sysfs_dirent * sd = dentry->d_fsdata; struct sysfs_inode_attrs *sd_attrs; struct iattr *iattrs; unsigned int ia_valid = iattr->ia_valid; - int error; - - if (!sd) - return -EINVAL; sd_attrs = sd->s_iattr; - error = inode_change_ok(inode, iattr); - if (error) - return error; - - iattr->ia_valid &= ~ATTR_SIZE; /* ignore size changes */ - - error = inode_setattr(inode, iattr); - if (error) - return error; - if (!sd_attrs) { /* setting attributes for the first time, allocate now */ sd_attrs = sysfs_init_inode_attrs(sd); @@ -111,12 +95,39 @@ int sysfs_setattr(struct dentry * dentry, struct iattr * iattr) iattrs->ia_ctime = iattr->ia_ctime; if (ia_valid & ATTR_MODE) { umode_t mode = iattr->ia_mode; - - if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID)) - mode &= ~S_ISGID; iattrs->ia_mode = sd->s_mode = mode; } } + return 0; +} + +int sysfs_setattr(struct dentry *dentry, struct iattr *iattr) +{ + struct inode *inode = dentry->d_inode; + struct sysfs_dirent *sd = dentry->d_fsdata; + int error; + + if (!sd) + return -EINVAL; + + error = inode_change_ok(inode, iattr); + if (error) + return error; + + iattr->ia_valid &= ~ATTR_SIZE; /* ignore size changes */ + if (iattr->ia_valid & ATTR_MODE) { + if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID)) + iattr->ia_mode &= ~S_ISGID; + } + + error = inode_setattr(inode, iattr); + if (error) + return error; + + mutex_lock(&sysfs_mutex); + error = sysfs_sd_setattr(sd, iattr); + mutex_unlock(&sysfs_mutex); + return error; } diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index af4c4e7482ac..a96d9678ae73 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -155,6 +155,7 @@ static inline void __sysfs_put(struct sysfs_dirent *sd) */ struct inode *sysfs_get_inode(struct sysfs_dirent *sd); void sysfs_delete_inode(struct inode *inode); +int sysfs_sd_setattr(struct sysfs_dirent *sd, struct iattr *iattr); int sysfs_setattr(struct dentry *dentry, struct iattr *iattr); int sysfs_setxattr(struct dentry *dentry, const char *name, const void *value, size_t size, int flags); -- cgit v1.2.3 From 6b0bfe9383a54cf35046980c61f5ff8fefa557d7 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 20 Nov 2009 16:08:51 -0800 Subject: sysfs: Update s_iattr on link and unlink. Currently sysfs updates the timestamps on the vfs directory inode when we create or remove a directory entry but doesn't update the cached copy on the sysfs_dirent, fix that oversight. Acked-by: Tejun Heo Acked-by: Serge Hallyn Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'fs') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index 9ee130be03be..36ee6d817956 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -464,6 +464,8 @@ void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt, */ int __sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd) { + struct sysfs_inode_attrs *ps_iattr; + if (sysfs_find_dirent(acxt->parent_sd, sd->s_name)) return -EEXIST; @@ -476,6 +478,13 @@ int __sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd) sysfs_link_sibling(sd); + /* Update timestamps on the parent */ + ps_iattr = acxt->parent_sd->s_iattr; + if (ps_iattr) { + struct iattr *ps_iattrs = &ps_iattr->ia_iattr; + ps_iattrs->ia_ctime = ps_iattrs->ia_mtime = CURRENT_TIME; + } + return 0; } @@ -554,10 +563,19 @@ int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd) */ void sysfs_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd) { + struct sysfs_inode_attrs *ps_iattr; + BUG_ON(sd->s_flags & SYSFS_FLAG_REMOVED); sysfs_unlink_sibling(sd); + /* Update timestamps on the parent */ + ps_iattr = acxt->parent_sd->s_iattr; + if (ps_iattr) { + struct iattr *ps_iattrs = &ps_iattr->ia_iattr; + ps_iattrs->ia_ctime = ps_iattrs->ia_mtime = CURRENT_TIME; + } + sd->s_flags |= SYSFS_FLAG_REMOVED; sd->s_sibling = acxt->removed; acxt->removed = sd; -- cgit v1.2.3 From c099aacd48ee73bd2de7da029e536ed005d72a43 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 20 Nov 2009 16:08:52 -0800 Subject: sysfs: Nicely indent sysfs_symlink_inode_operations Lining up the functions in sysfs_symlink_inode_operations follows the pattern in the rest of sysfs and makes things slightly more readable. Acked-by: Tejun Heo Acked-by: Serge Hallyn Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/symlink.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c index c5081ad77026..11374182ca67 100644 --- a/fs/sysfs/symlink.c +++ b/fs/sysfs/symlink.c @@ -210,10 +210,10 @@ static void sysfs_put_link(struct dentry *dentry, struct nameidata *nd, void *co } const struct inode_operations sysfs_symlink_inode_operations = { - .setxattr = sysfs_setxattr, - .readlink = generic_readlink, - .follow_link = sysfs_follow_link, - .put_link = sysfs_put_link, + .setxattr = sysfs_setxattr, + .readlink = generic_readlink, + .follow_link = sysfs_follow_link, + .put_link = sysfs_put_link, }; -- cgit v1.2.3 From e61ab4ae48fbf477f5b9fcbec9e1b8dc789920d0 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 20 Nov 2009 16:08:53 -0800 Subject: sysfs: Implement sysfs_getattr & sysfs_permission With the implementation of sysfs_getattr and sysfs_permission sysfs becomes able to lazily propogate inode attribute changes from the sysfs_dirents to the vfs inodes. This paves the way for deleting significant chunks of now unnecessary code. While doing this we did not reference sysfs_setattr from sysfs_symlink_inode_operations so I added along with sysfs_getattr and sysfs_permission. Acked-by: Tejun Heo Acked-by: Serge Hallyn Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 2 ++ fs/sysfs/inode.c | 64 +++++++++++++++++++++++++++++++++++++++--------------- fs/sysfs/symlink.c | 3 +++ fs/sysfs/sysfs.h | 2 ++ 4 files changed, 54 insertions(+), 17 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index 36ee6d817956..e319379d36db 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -800,7 +800,9 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry, const struct inode_operations sysfs_dir_inode_operations = { .lookup = sysfs_lookup, + .permission = sysfs_permission, .setattr = sysfs_setattr, + .getattr = sysfs_getattr, .setxattr = sysfs_setxattr, }; diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c index 76e977c300f7..1ffd5559926f 100644 --- a/fs/sysfs/inode.c +++ b/fs/sysfs/inode.c @@ -37,7 +37,9 @@ static struct backing_dev_info sysfs_backing_dev_info = { }; static const struct inode_operations sysfs_inode_operations ={ + .permission = sysfs_permission, .setattr = sysfs_setattr, + .getattr = sysfs_getattr, .setxattr = sysfs_setxattr, }; @@ -196,7 +198,6 @@ static inline void set_default_inode_attr(struct inode * inode, mode_t mode) static inline void set_inode_attr(struct inode * inode, struct iattr * iattr) { - inode->i_mode = iattr->ia_mode; inode->i_uid = iattr->ia_uid; inode->i_gid = iattr->ia_gid; inode->i_atime = iattr->ia_atime; @@ -227,38 +228,56 @@ static int sysfs_count_nlink(struct sysfs_dirent *sd) return nr + 2; } +static void sysfs_refresh_inode(struct sysfs_dirent *sd, struct inode *inode) +{ + struct sysfs_inode_attrs *iattrs = sd->s_iattr; + + inode->i_mode = sd->s_mode; + if (iattrs) { + /* sysfs_dirent has non-default attributes + * get them from persistent copy in sysfs_dirent + */ + set_inode_attr(inode, &iattrs->ia_iattr); + security_inode_notifysecctx(inode, + iattrs->ia_secdata, + iattrs->ia_secdata_len); + } + + if (sysfs_type(sd) == SYSFS_DIR) + inode->i_nlink = sysfs_count_nlink(sd); +} + +int sysfs_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat) +{ + struct sysfs_dirent *sd = dentry->d_fsdata; + struct inode *inode = dentry->d_inode; + + mutex_lock(&sysfs_mutex); + sysfs_refresh_inode(sd, inode); + mutex_unlock(&sysfs_mutex); + + generic_fillattr(inode, stat); + return 0; +} + static void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode) { struct bin_attribute *bin_attr; - struct sysfs_inode_attrs *iattrs; inode->i_private = sysfs_get(sd); inode->i_mapping->a_ops = &sysfs_aops; inode->i_mapping->backing_dev_info = &sysfs_backing_dev_info; inode->i_op = &sysfs_inode_operations; - inode->i_ino = sd->s_ino; lockdep_set_class(&inode->i_mutex, &sysfs_inode_imutex_key); - iattrs = sd->s_iattr; - if (iattrs) { - /* sysfs_dirent has non-default attributes - * get them for the new inode from persistent copy - * in sysfs_dirent - */ - set_inode_attr(inode, &iattrs->ia_iattr); - if (iattrs->ia_secdata) - security_inode_notifysecctx(inode, - iattrs->ia_secdata, - iattrs->ia_secdata_len); - } else - set_default_inode_attr(inode, sd->s_mode); + set_default_inode_attr(inode, sd->s_mode); + sysfs_refresh_inode(sd, inode); /* initialize inode according to type */ switch (sysfs_type(sd)) { case SYSFS_DIR: inode->i_op = &sysfs_dir_inode_operations; inode->i_fop = &sysfs_dir_operations; - inode->i_nlink = sysfs_count_nlink(sd); break; case SYSFS_KOBJ_ATTR: inode->i_size = PAGE_SIZE; @@ -341,3 +360,14 @@ int sysfs_hash_and_remove(struct sysfs_dirent *dir_sd, const char *name) else return -ENOENT; } + +int sysfs_permission(struct inode *inode, int mask) +{ + struct sysfs_dirent *sd = inode->i_private; + + mutex_lock(&sysfs_mutex); + sysfs_refresh_inode(sd, inode); + mutex_unlock(&sysfs_mutex); + + return generic_permission(inode, mask, NULL); +} diff --git a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c index 11374182ca67..c5eff49fa41b 100644 --- a/fs/sysfs/symlink.c +++ b/fs/sysfs/symlink.c @@ -214,6 +214,9 @@ const struct inode_operations sysfs_symlink_inode_operations = { .readlink = generic_readlink, .follow_link = sysfs_follow_link, .put_link = sysfs_put_link, + .setattr = sysfs_setattr, + .getattr = sysfs_getattr, + .permission = sysfs_permission, }; diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index a96d9678ae73..12ccc07459d8 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -156,7 +156,9 @@ static inline void __sysfs_put(struct sysfs_dirent *sd) struct inode *sysfs_get_inode(struct sysfs_dirent *sd); void sysfs_delete_inode(struct inode *inode); int sysfs_sd_setattr(struct sysfs_dirent *sd, struct iattr *iattr); +int sysfs_permission(struct inode *inode, int mask); int sysfs_setattr(struct dentry *dentry, struct iattr *iattr); +int sysfs_getattr(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat); int sysfs_setxattr(struct dentry *dentry, const char *name, const void *value, size_t size, int flags); int sysfs_hash_and_remove(struct sysfs_dirent *dir_sd, const char *name); -- cgit v1.2.3 From 06fc0d66f7ed3a3b08e8fcf8c325ecf0b8f93fea Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 20 Nov 2009 16:08:54 -0800 Subject: sysfs: In sysfs_chmod_file lazily propagate the mode change. Now that sysfs_getattr and sysfs_permission refresh the vfs inode there is no need to immediatly push the mode change into the vfs cache. Reducing the amount of work needed and simplifying the locking. Acked-by: Tejun Heo Acked-by: Serge Hallyn Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/file.c | 31 ++++++++----------------------- 1 file changed, 8 insertions(+), 23 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c index faa1a803caa9..dc30d9e31683 100644 --- a/fs/sysfs/file.c +++ b/fs/sysfs/file.c @@ -579,38 +579,23 @@ EXPORT_SYMBOL_GPL(sysfs_add_file_to_group); */ int sysfs_chmod_file(struct kobject *kobj, struct attribute *attr, mode_t mode) { - struct sysfs_dirent *victim_sd = NULL; - struct dentry *victim = NULL; - struct inode * inode; + struct sysfs_dirent *sd; struct iattr newattrs; int rc; - rc = -ENOENT; - victim_sd = sysfs_get_dirent(kobj->sd, attr->name); - if (!victim_sd) - goto out; + mutex_lock(&sysfs_mutex); - mutex_lock(&sysfs_rename_mutex); - victim = sysfs_get_dentry(victim_sd); - mutex_unlock(&sysfs_rename_mutex); - if (IS_ERR(victim)) { - rc = PTR_ERR(victim); - victim = NULL; + rc = -ENOENT; + sd = sysfs_find_dirent(kobj->sd, attr->name); + if (!sd) goto out; - } - - inode = victim->d_inode; - mutex_lock(&inode->i_mutex); - - newattrs.ia_mode = (mode & S_IALLUGO) | (inode->i_mode & ~S_IALLUGO); + newattrs.ia_mode = (mode & S_IALLUGO) | (sd->s_mode & ~S_IALLUGO); newattrs.ia_valid = ATTR_MODE; - rc = sysfs_setattr(victim, &newattrs); + rc = sysfs_sd_setattr(sd, &newattrs); - mutex_unlock(&inode->i_mutex); out: - dput(victim); - sysfs_put(victim_sd); + mutex_unlock(&sysfs_mutex); return rc; } EXPORT_SYMBOL_GPL(sysfs_chmod_file); -- cgit v1.2.3 From a16bbc3430ed94b543222f4c8ef68025f8493e93 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 20 Nov 2009 16:08:55 -0800 Subject: sysfs: Gut sysfs_addrm_start and sysfs_addrm_finish With lazy inode updates and dentry operations bringing everything into sync on demand there is no longer any need to immediately update the vfs or grab i_mutex to protect those updates as we make changes to sysfs. Acked-by: Serge Hallyn Acked-by: Tejun Heo Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 91 +++----------------------------------------------------- fs/sysfs/sysfs.h | 2 -- 2 files changed, 4 insertions(+), 89 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index e319379d36db..f3af45e47eaa 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -386,12 +386,6 @@ struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type) return NULL; } -static int sysfs_ilookup_test(struct inode *inode, void *arg) -{ - struct sysfs_dirent *sd = arg; - return inode->i_ino == sd->s_ino; -} - /** * sysfs_addrm_start - prepare for sysfs_dirent add/remove * @acxt: pointer to sysfs_addrm_cxt to be used @@ -399,47 +393,20 @@ static int sysfs_ilookup_test(struct inode *inode, void *arg) * * This function is called when the caller is about to add or * remove sysfs_dirent under @parent_sd. This function acquires - * sysfs_mutex, grabs inode for @parent_sd if available and lock - * i_mutex of it. @acxt is used to keep and pass context to + * sysfs_mutex. @acxt is used to keep and pass context to * other addrm functions. * * LOCKING: * Kernel thread context (may sleep). sysfs_mutex is locked on - * return. i_mutex of parent inode is locked on return if - * available. + * return. */ void sysfs_addrm_start(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *parent_sd) { - struct inode *inode; - memset(acxt, 0, sizeof(*acxt)); acxt->parent_sd = parent_sd; - /* Lookup parent inode. inode initialization is protected by - * sysfs_mutex, so inode existence can be determined by - * looking up inode while holding sysfs_mutex. - */ mutex_lock(&sysfs_mutex); - - inode = ilookup5(sysfs_sb, parent_sd->s_ino, sysfs_ilookup_test, - parent_sd); - if (inode) { - WARN_ON(inode->i_state & I_NEW); - - /* parent inode available */ - acxt->parent_inode = inode; - - /* sysfs_mutex is below i_mutex in lock hierarchy. - * First, trylock i_mutex. If fails, unlock - * sysfs_mutex and lock them in order. - */ - if (!mutex_trylock(&inode->i_mutex)) { - mutex_unlock(&sysfs_mutex); - mutex_lock(&inode->i_mutex); - mutex_lock(&sysfs_mutex); - } - } } /** @@ -471,11 +438,6 @@ int __sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd) sd->s_parent = sysfs_get(acxt->parent_sd); - if (sysfs_type(sd) == SYSFS_DIR && acxt->parent_inode) - inc_nlink(acxt->parent_inode); - - acxt->cnt++; - sysfs_link_sibling(sd); /* Update timestamps on the parent */ @@ -579,40 +541,6 @@ void sysfs_remove_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd) sd->s_flags |= SYSFS_FLAG_REMOVED; sd->s_sibling = acxt->removed; acxt->removed = sd; - - if (sysfs_type(sd) == SYSFS_DIR && acxt->parent_inode) - drop_nlink(acxt->parent_inode); - - acxt->cnt++; -} - -/** - * sysfs_dec_nlink - Decrement link count for the specified sysfs_dirent - * @sd: target sysfs_dirent - * - * Decrement nlink for @sd. @sd must have been unlinked from its - * parent on entry to this function such that it can't be looked - * up anymore. - */ -static void sysfs_dec_nlink(struct sysfs_dirent *sd) -{ - struct inode *inode; - - inode = ilookup(sysfs_sb, sd->s_ino); - if (!inode) - return; - - /* adjust nlink and update timestamp */ - mutex_lock(&inode->i_mutex); - - inode->i_ctime = CURRENT_TIME; - drop_nlink(inode); - if (sysfs_type(sd) == SYSFS_DIR) - drop_nlink(inode); - - mutex_unlock(&inode->i_mutex); - - iput(inode); } /** @@ -621,25 +549,15 @@ static void sysfs_dec_nlink(struct sysfs_dirent *sd) * * Finish up sysfs_dirent add/remove. Resources acquired by * sysfs_addrm_start() are released and removed sysfs_dirents are - * cleaned up. Timestamps on the parent inode are updated. + * cleaned up. * * LOCKING: - * All mutexes acquired by sysfs_addrm_start() are released. + * sysfs_mutex is released. */ void sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt) { /* release resources acquired by sysfs_addrm_start() */ mutex_unlock(&sysfs_mutex); - if (acxt->parent_inode) { - struct inode *inode = acxt->parent_inode; - - /* if added/removed, update timestamps on the parent */ - if (acxt->cnt) - inode->i_ctime = inode->i_mtime = CURRENT_TIME; - - mutex_unlock(&inode->i_mutex); - iput(inode); - } /* kill removed sysfs_dirents */ while (acxt->removed) { @@ -648,7 +566,6 @@ void sysfs_addrm_finish(struct sysfs_addrm_cxt *acxt) acxt->removed = sd->s_sibling; sd->s_sibling = NULL; - sysfs_dec_nlink(sd); sysfs_deactivate(sd); unmap_bin_file(sd); sysfs_put(sd); diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index 12ccc07459d8..90b35012abb2 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -89,9 +89,7 @@ static inline unsigned int sysfs_type(struct sysfs_dirent *sd) */ struct sysfs_addrm_cxt { struct sysfs_dirent *parent_sd; - struct inode *parent_inode; struct sysfs_dirent *removed; - int cnt; }; /* -- cgit v1.2.3 From 832b6af198aefe6034310e124594cc8b833c0ef9 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 20 Nov 2009 16:08:56 -0800 Subject: sysfs: Propagate renames to the vfs on demand By teaching sysfs_revalidate to hide a dentry for a sysfs_dirent if the sysfs_dirent has been renamed, and by teaching sysfs_lookup to return the original dentry if the sysfs dirent has been renamed. I can show the results of renames correctly without having to update the dcache during the directory rename. This massively simplifies the rename logic allowing a lot of weird sysfs special cases to be removed along with a lot of now unnecesary helper code. Acked-by: Tejun Heo Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/namei.c | 22 ------- fs/sysfs/dir.c | 158 ++++++++++---------------------------------------- fs/sysfs/inode.c | 12 ---- fs/sysfs/sysfs.h | 1 - include/linux/namei.h | 1 - 5 files changed, 32 insertions(+), 162 deletions(-) (limited to 'fs') diff --git a/fs/namei.c b/fs/namei.c index d11f404667e9..d3c190c35fcc 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -1279,28 +1279,6 @@ struct dentry *lookup_one_len(const char *name, struct dentry *base, int len) return __lookup_hash(&this, base, NULL); } -/** - * lookup_one_noperm - bad hack for sysfs - * @name: pathname component to lookup - * @base: base directory to lookup from - * - * This is a variant of lookup_one_len that doesn't perform any permission - * checks. It's a horrible hack to work around the braindead sysfs - * architecture and should not be used anywhere else. - * - * DON'T USE THIS FUNCTION EVER, thanks. - */ -struct dentry *lookup_one_noperm(const char *name, struct dentry *base) -{ - int err; - struct qstr this; - - err = __lookup_one_len(name, &this, base, strlen(name)); - if (err) - return ERR_PTR(err); - return __lookup_hash(&this, base, NULL); -} - int user_path_at(int dfd, const char __user *name, unsigned flags, struct path *path) { diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index f3af45e47eaa..97954c69ff0e 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -25,7 +25,6 @@ #include "sysfs.h" DEFINE_MUTEX(sysfs_mutex); -DEFINE_MUTEX(sysfs_rename_mutex); DEFINE_SPINLOCK(sysfs_assoc_lock); static DEFINE_SPINLOCK(sysfs_ino_lock); @@ -84,46 +83,6 @@ static void sysfs_unlink_sibling(struct sysfs_dirent *sd) } } -/** - * sysfs_get_dentry - get dentry for the given sysfs_dirent - * @sd: sysfs_dirent of interest - * - * Get dentry for @sd. Dentry is looked up if currently not - * present. This function descends from the root looking up - * dentry for each step. - * - * LOCKING: - * mutex_lock(sysfs_rename_mutex) - * - * RETURNS: - * Pointer to found dentry on success, ERR_PTR() value on error. - */ -struct dentry *sysfs_get_dentry(struct sysfs_dirent *sd) -{ - struct dentry *dentry = dget(sysfs_sb->s_root); - - while (dentry->d_fsdata != sd) { - struct sysfs_dirent *cur; - struct dentry *parent; - - /* find the first ancestor which hasn't been looked up */ - cur = sd; - while (cur->s_parent != dentry->d_fsdata) - cur = cur->s_parent; - - /* look it up */ - parent = dentry; - mutex_lock(&parent->d_inode->i_mutex); - dentry = lookup_one_noperm(cur->s_name, parent); - mutex_unlock(&parent->d_inode->i_mutex); - dput(parent); - - if (IS_ERR(dentry)) - break; - } - return dentry; -} - /** * sysfs_get_active - get an active reference to sysfs_dirent * @sd: sysfs_dirent to get an active reference to @@ -315,6 +274,14 @@ static int sysfs_dentry_revalidate(struct dentry *dentry, struct nameidata *nd) if (sd->s_flags & SYSFS_FLAG_REMOVED) goto out_bad; + /* The sysfs dirent has been moved? */ + if (dentry->d_parent->d_fsdata != sd->s_parent) + goto out_bad; + + /* The sysfs dirent has been renamed */ + if (strcmp(dentry->d_name.name, sd->s_name) != 0) + goto out_bad; + mutex_unlock(&sysfs_mutex); out_valid: return 1; @@ -322,6 +289,12 @@ out_bad: /* Remove the dentry from the dcache hashes. * If this is a deleted dentry we use d_drop instead of d_delete * so sysfs doesn't need to cope with negative dentries. + * + * If this is a dentry that has simply been renamed we + * use d_drop to remove it from the dcache lookup on its + * old parent. If this dentry persists later when a lookup + * is performed at its new name the dentry will be readded + * to the dcache hashes. */ is_dir = (sysfs_type(sd) == SYSFS_DIR); mutex_unlock(&sysfs_mutex); @@ -705,10 +678,15 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry, } /* instantiate and hash dentry */ - dentry->d_op = &sysfs_dentry_ops; - dentry->d_fsdata = sysfs_get(sd); - d_instantiate(dentry, inode); - d_rehash(dentry); + ret = d_find_alias(inode); + if (!ret) { + dentry->d_op = &sysfs_dentry_ops; + dentry->d_fsdata = sysfs_get(sd); + d_add(dentry, inode); + } else { + d_move(ret, dentry); + iput(inode); + } out_unlock: mutex_unlock(&sysfs_mutex); @@ -785,62 +763,32 @@ void sysfs_remove_dir(struct kobject * kobj) int sysfs_rename_dir(struct kobject * kobj, const char *new_name) { struct sysfs_dirent *sd = kobj->sd; - struct dentry *parent = NULL; - struct dentry *old_dentry = NULL, *new_dentry = NULL; const char *dup_name = NULL; int error; - mutex_lock(&sysfs_rename_mutex); + mutex_lock(&sysfs_mutex); error = 0; if (strcmp(sd->s_name, new_name) == 0) goto out; /* nothing to rename */ - /* get the original dentry */ - old_dentry = sysfs_get_dentry(sd); - if (IS_ERR(old_dentry)) { - error = PTR_ERR(old_dentry); - old_dentry = NULL; - goto out; - } - - parent = old_dentry->d_parent; - - /* lock parent and get dentry for new name */ - mutex_lock(&parent->d_inode->i_mutex); - mutex_lock(&sysfs_mutex); - error = -EEXIST; if (sysfs_find_dirent(sd->s_parent, new_name)) - goto out_unlock; - - error = -ENOMEM; - new_dentry = d_alloc_name(parent, new_name); - if (!new_dentry) - goto out_unlock; + goto out; /* rename sysfs_dirent */ error = -ENOMEM; new_name = dup_name = kstrdup(new_name, GFP_KERNEL); if (!new_name) - goto out_unlock; + goto out; dup_name = sd->s_name; sd->s_name = new_name; - /* rename */ - d_add(new_dentry, NULL); - d_move(old_dentry, new_dentry); - error = 0; - out_unlock: + out: mutex_unlock(&sysfs_mutex); - mutex_unlock(&parent->d_inode->i_mutex); kfree(dup_name); - dput(old_dentry); - dput(new_dentry); - out: - mutex_unlock(&sysfs_rename_mutex); return error; } @@ -848,12 +796,11 @@ int sysfs_move_dir(struct kobject *kobj, struct kobject *new_parent_kobj) { struct sysfs_dirent *sd = kobj->sd; struct sysfs_dirent *new_parent_sd; - struct dentry *old_parent, *new_parent = NULL; - struct dentry *old_dentry = NULL, *new_dentry = NULL; int error; - mutex_lock(&sysfs_rename_mutex); BUG_ON(!sd->s_parent); + + mutex_lock(&sysfs_mutex); new_parent_sd = (new_parent_kobj && new_parent_kobj->sd) ? new_parent_kobj->sd : &sysfs_root; @@ -861,61 +808,20 @@ int sysfs_move_dir(struct kobject *kobj, struct kobject *new_parent_kobj) if (sd->s_parent == new_parent_sd) goto out; /* nothing to move */ - /* get dentries */ - old_dentry = sysfs_get_dentry(sd); - if (IS_ERR(old_dentry)) { - error = PTR_ERR(old_dentry); - old_dentry = NULL; - goto out; - } - old_parent = old_dentry->d_parent; - - new_parent = sysfs_get_dentry(new_parent_sd); - if (IS_ERR(new_parent)) { - error = PTR_ERR(new_parent); - new_parent = NULL; - goto out; - } - -again: - mutex_lock(&old_parent->d_inode->i_mutex); - if (!mutex_trylock(&new_parent->d_inode->i_mutex)) { - mutex_unlock(&old_parent->d_inode->i_mutex); - goto again; - } - mutex_lock(&sysfs_mutex); - error = -EEXIST; if (sysfs_find_dirent(new_parent_sd, sd->s_name)) - goto out_unlock; - - error = -ENOMEM; - new_dentry = d_alloc_name(new_parent, sd->s_name); - if (!new_dentry) - goto out_unlock; - - error = 0; - d_add(new_dentry, NULL); - d_move(old_dentry, new_dentry); + goto out; /* Remove from old parent's list and insert into new parent's list. */ sysfs_unlink_sibling(sd); sysfs_get(new_parent_sd); - drop_nlink(old_parent->d_inode); sysfs_put(sd->s_parent); sd->s_parent = new_parent_sd; - inc_nlink(new_parent->d_inode); sysfs_link_sibling(sd); - out_unlock: + error = 0; +out: mutex_unlock(&sysfs_mutex); - mutex_unlock(&new_parent->d_inode->i_mutex); - mutex_unlock(&old_parent->d_inode->i_mutex); - out: - dput(new_parent); - dput(old_dentry); - dput(new_dentry); - mutex_unlock(&sysfs_rename_mutex); return error; } diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c index 1ffd5559926f..9f783d4e4b51 100644 --- a/fs/sysfs/inode.c +++ b/fs/sysfs/inode.c @@ -205,17 +205,6 @@ static inline void set_inode_attr(struct inode * inode, struct iattr * iattr) inode->i_ctime = iattr->ia_ctime; } - -/* - * sysfs has a different i_mutex lock order behavior for i_mutex than other - * filesystems; sysfs i_mutex is called in many places with subsystem locks - * held. At the same time, many of the VFS locking rules do not apply to - * sysfs at all (cross directory rename for example). To untangle this mess - * (which gives false positives in lockdep), we're giving sysfs inodes their - * own class for i_mutex. - */ -static struct lock_class_key sysfs_inode_imutex_key; - static int sysfs_count_nlink(struct sysfs_dirent *sd) { struct sysfs_dirent *child; @@ -268,7 +257,6 @@ static void sysfs_init_inode(struct sysfs_dirent *sd, struct inode *inode) inode->i_mapping->a_ops = &sysfs_aops; inode->i_mapping->backing_dev_info = &sysfs_backing_dev_info; inode->i_op = &sysfs_inode_operations; - lockdep_set_class(&inode->i_mutex, &sysfs_inode_imutex_key); set_default_inode_attr(inode, sd->s_mode); sysfs_refresh_inode(sd, inode); diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index 90b35012abb2..98a15bf1efe1 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -103,7 +103,6 @@ extern struct kmem_cache *sysfs_dir_cachep; * dir.c */ extern struct mutex sysfs_mutex; -extern struct mutex sysfs_rename_mutex; extern spinlock_t sysfs_assoc_lock; extern const struct file_operations sysfs_dir_operations; diff --git a/include/linux/namei.h b/include/linux/namei.h index ec0f607b364a..028946750289 100644 --- a/include/linux/namei.h +++ b/include/linux/namei.h @@ -76,7 +76,6 @@ extern struct file *nameidata_to_filp(struct nameidata *nd, int flags); extern void release_open_intent(struct nameidata *); extern struct dentry *lookup_one_len(const char *, struct dentry *, int); -extern struct dentry *lookup_one_noperm(const char *, struct dentry *); extern int follow_down(struct path *); extern int follow_up(struct path *); -- cgit v1.2.3 From ca1bab38195d66bdf42320a99cc7359434a271d3 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 20 Nov 2009 16:08:57 -0800 Subject: sysfs: Factor out sysfs_rename from sysfs_rename_dir and sysfs_move_dir These two functions do 90% of the same work and it doesn't significantly obfuscate the function to allow both the parent dir and the name to change at the same time. So merge them together to simplify maintenance, and increase testing. Acked-by: Tejun Heo Acked-by: Serge Hallyn Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 62 ++++++++++++++++++++++++++------------------------------ fs/sysfs/sysfs.h | 3 +++ 2 files changed, 32 insertions(+), 33 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index 97954c69ff0e..f05f2303a8b8 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -760,30 +760,42 @@ void sysfs_remove_dir(struct kobject * kobj) __sysfs_remove_dir(sd); } -int sysfs_rename_dir(struct kobject * kobj, const char *new_name) +int sysfs_rename(struct sysfs_dirent *sd, + struct sysfs_dirent *new_parent_sd, const char *new_name) { - struct sysfs_dirent *sd = kobj->sd; const char *dup_name = NULL; int error; mutex_lock(&sysfs_mutex); error = 0; - if (strcmp(sd->s_name, new_name) == 0) + if ((sd->s_parent == new_parent_sd) && + (strcmp(sd->s_name, new_name) == 0)) goto out; /* nothing to rename */ error = -EEXIST; - if (sysfs_find_dirent(sd->s_parent, new_name)) + if (sysfs_find_dirent(new_parent_sd, new_name)) goto out; /* rename sysfs_dirent */ - error = -ENOMEM; - new_name = dup_name = kstrdup(new_name, GFP_KERNEL); - if (!new_name) - goto out; + if (strcmp(sd->s_name, new_name) != 0) { + error = -ENOMEM; + new_name = dup_name = kstrdup(new_name, GFP_KERNEL); + if (!new_name) + goto out; + + dup_name = sd->s_name; + sd->s_name = new_name; + } - dup_name = sd->s_name; - sd->s_name = new_name; + /* Remove from old parent's list and insert into new parent's list. */ + if (sd->s_parent != new_parent_sd) { + sysfs_unlink_sibling(sd); + sysfs_get(new_parent_sd); + sysfs_put(sd->s_parent); + sd->s_parent = new_parent_sd; + sysfs_link_sibling(sd); + } error = 0; out: @@ -792,37 +804,21 @@ int sysfs_rename_dir(struct kobject * kobj, const char *new_name) return error; } +int sysfs_rename_dir(struct kobject *kobj, const char *new_name) +{ + return sysfs_rename(kobj->sd, kobj->sd->s_parent, new_name); +} + int sysfs_move_dir(struct kobject *kobj, struct kobject *new_parent_kobj) { struct sysfs_dirent *sd = kobj->sd; struct sysfs_dirent *new_parent_sd; - int error; BUG_ON(!sd->s_parent); - - mutex_lock(&sysfs_mutex); - new_parent_sd = (new_parent_kobj && new_parent_kobj->sd) ? + new_parent_sd = new_parent_kobj && new_parent_kobj->sd ? new_parent_kobj->sd : &sysfs_root; - error = 0; - if (sd->s_parent == new_parent_sd) - goto out; /* nothing to move */ - - error = -EEXIST; - if (sysfs_find_dirent(new_parent_sd, sd->s_name)) - goto out; - - /* Remove from old parent's list and insert into new parent's list. */ - sysfs_unlink_sibling(sd); - sysfs_get(new_parent_sd); - sysfs_put(sd->s_parent); - sd->s_parent = new_parent_sd; - sysfs_link_sibling(sd); - - error = 0; -out: - mutex_unlock(&sysfs_mutex); - return error; + return sysfs_rename(sd, new_parent_sd, sd->s_name); } /* Relationship between s_mode and the DT_xxx types */ diff --git a/fs/sysfs/sysfs.h b/fs/sysfs/sysfs.h index 98a15bf1efe1..ca52e7b9d8f8 100644 --- a/fs/sysfs/sysfs.h +++ b/fs/sysfs/sysfs.h @@ -130,6 +130,9 @@ int sysfs_create_subdir(struct kobject *kobj, const char *name, struct sysfs_dirent **p_sd); void sysfs_remove_subdir(struct sysfs_dirent *sd); +int sysfs_rename(struct sysfs_dirent *sd, + struct sysfs_dirent *new_parent_sd, const char *new_name); + static inline struct sysfs_dirent *__sysfs_get(struct sysfs_dirent *sd) { if (sd) { -- cgit v1.2.3 From e16acb503b42ef241a9396de7c5a1614c74d8ca6 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Fri, 20 Nov 2009 16:08:58 -0800 Subject: sysfs: sysfs_setattr remove unnecessary permission check. inode_change_ok already clears the SGID bit when necessary so there is no reason for sysfs_setattr to carry code to do the same, and it is good to kill the extra copy because when I moved the code last in certain corner cases the code will look at the wrong gid. Acked-by: Serge Hallyn Acked-by: Tejun Heo Signed-off-by: Eric W. Biederman Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/inode.c | 4 ---- 1 file changed, 4 deletions(-) (limited to 'fs') diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c index 9f783d4e4b51..220b758523ae 100644 --- a/fs/sysfs/inode.c +++ b/fs/sysfs/inode.c @@ -117,10 +117,6 @@ int sysfs_setattr(struct dentry *dentry, struct iattr *iattr) return error; iattr->ia_valid &= ~ATTR_SIZE; /* ignore size changes */ - if (iattr->ia_valid & ATTR_MODE) { - if (!in_group_p(inode->i_gid) && !capable(CAP_FSETID)) - iattr->ia_mode &= ~S_ISGID; - } error = inode_setattr(inode, iattr); if (error) -- cgit v1.2.3