From 0b728e1911cbe6e24020727c3870628b9653f32a Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 10 Jun 2012 16:03:43 -0400 Subject: stop passing nameidata * to ->d_revalidate() Just the lookup flags. Die, bastard, die... Signed-off-by: Al Viro --- fs/sysfs/dir.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/sysfs/dir.c') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index e6bb9b2a4cb..038e74b3af8 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -303,12 +303,12 @@ static int sysfs_dentry_delete(const struct dentry *dentry) return !!(sd->s_flags & SYSFS_FLAG_REMOVED); } -static int sysfs_dentry_revalidate(struct dentry *dentry, struct nameidata *nd) +static int sysfs_dentry_revalidate(struct dentry *dentry, unsigned int flags) { struct sysfs_dirent *sd; int is_dir; - if (nd->flags & LOOKUP_RCU) + if (flags & LOOKUP_RCU) return -ECHILD; sd = dentry->d_fsdata; -- cgit v1.2.3 From 00cd8dd3bf95f2cc8435b4cac01d9995635c6d0b Mon Sep 17 00:00:00 2001 From: Al Viro Date: Sun, 10 Jun 2012 17:13:09 -0400 Subject: stop passing nameidata to ->lookup() Just the flags; only NFS cares even about that, but there are legitimate uses for such argument. And getting rid of that completely would require splitting ->lookup() into a couple of methods (at least), so let's leave that alone for now... Signed-off-by: Al Viro --- fs/sysfs/dir.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/sysfs/dir.c') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index 038e74b3af8..efd373e3e0a 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -764,7 +764,7 @@ int sysfs_create_dir(struct kobject * kobj) } static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry, - struct nameidata *nd) + unsigned int flags) { struct dentry *ret = NULL; struct dentry *parent = dentry->d_parent; -- cgit v1.2.3 From 469796d10590341c53cff0a2959254eaf5d465de Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 7 Jun 2012 20:51:39 -0400 Subject: sysfs: switch to ->s_d_op and ->d_release() a) ->d_iput() is wrong here - what we do to inode is completely usual, it's dentry->d_fsdata that we want to drop. Just use ->d_release(). b) switch to ->s_d_op - no need to play with d_set_d_op() Signed-off-by: Al Viro --- fs/sysfs/dir.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) (limited to 'fs/sysfs/dir.c') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index efd373e3e0a..77c44ce493f 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -300,7 +300,7 @@ void release_sysfs_dirent(struct sysfs_dirent * sd) static int sysfs_dentry_delete(const struct dentry *dentry) { struct sysfs_dirent *sd = dentry->d_fsdata; - return !!(sd->s_flags & SYSFS_FLAG_REMOVED); + return !(sd && !(sd->s_flags & SYSFS_FLAG_REMOVED)); } static int sysfs_dentry_revalidate(struct dentry *dentry, unsigned int flags) @@ -355,18 +355,15 @@ out_bad: return 0; } -static void sysfs_dentry_iput(struct dentry *dentry, struct inode *inode) +static void sysfs_dentry_release(struct dentry *dentry) { - struct sysfs_dirent * sd = dentry->d_fsdata; - - sysfs_put(sd); - iput(inode); + sysfs_put(dentry->d_fsdata); } -static const struct dentry_operations sysfs_dentry_ops = { +const struct dentry_operations sysfs_dentry_ops = { .d_revalidate = sysfs_dentry_revalidate, .d_delete = sysfs_dentry_delete, - .d_iput = sysfs_dentry_iput, + .d_release = sysfs_dentry_release, }; struct sysfs_dirent *sysfs_new_dirent(const char *name, umode_t mode, int type) @@ -786,6 +783,7 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry, ret = ERR_PTR(-ENOENT); goto out_unlock; } + dentry->d_fsdata = sysfs_get(sd); /* attach dentry and inode */ inode = sysfs_get_inode(dir->i_sb, sd); @@ -797,8 +795,6 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry, /* instantiate and hash dentry */ ret = d_find_alias(inode); if (!ret) { - d_set_d_op(dentry, &sysfs_dentry_ops); - dentry->d_fsdata = sysfs_get(sd); d_add(dentry, inode); } else { d_move(ret, dentry); -- cgit v1.2.3 From e77fb7cef87856d9d35f2f4d617d0b97148ee7c2 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Thu, 7 Jun 2012 20:56:54 -0400 Subject: sysfs: just use d_materialise_unique() same as for nfs et.al. Signed-off-by: Al Viro --- fs/sysfs/dir.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) (limited to 'fs/sysfs/dir.c') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index 77c44ce493f..a5cf784f9cc 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -793,14 +793,7 @@ static struct dentry * sysfs_lookup(struct inode *dir, struct dentry *dentry, } /* instantiate and hash dentry */ - ret = d_find_alias(inode); - if (!ret) { - d_add(dentry, inode); - } else { - d_move(ret, dentry); - iput(inode); - } - + ret = d_materialise_unique(dentry, inode); out_unlock: mutex_unlock(&sysfs_mutex); return ret; -- cgit v1.2.3 From e5bcac61472ca627241b394d439decd00bba3aea Mon Sep 17 00:00:00 2001 From: Glauber Costa Date: Fri, 6 Jul 2012 13:09:07 +0400 Subject: sysfs: fail dentry revalidation after namespace change When we change the namespace tag of a sysfs entry, the associated dentry is still kept around. readdir() will work correctly and not display the old entries, but open() will still succeed, so will reads and writes. This will no longer happen if sysfs is remounted, hinting that this is a cache-related problem. I am using the following sequence to demonstrate that: shell1: ip link add type veth unshare -nm shell2: ip link set veth1 cat /sys/devices/virtual/net/veth1/ifindex Before that patch, this will succeed (fail to fail). After it, it will correctly return an error. Differently from a normal rename, which we handle fine, changing the object namespace will keep it's path intact. So this check seems necessary as well. [ v2: get type from parent, as suggested by Eric Biederman ] Signed-off-by: Glauber Costa CC: Tejun Heo Reviewed-by: "Eric W. Biederman" Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'fs/sysfs/dir.c') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index e6bb9b2a4cb..c0bf38a21ca 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -307,6 +307,7 @@ static int sysfs_dentry_revalidate(struct dentry *dentry, struct nameidata *nd) { struct sysfs_dirent *sd; int is_dir; + int type; if (nd->flags & LOOKUP_RCU) return -ECHILD; @@ -326,6 +327,13 @@ static int sysfs_dentry_revalidate(struct dentry *dentry, struct nameidata *nd) if (strcmp(dentry->d_name.name, sd->s_name) != 0) goto out_bad; + /* The sysfs dirent has been moved to a different namespace */ + type = KOBJ_NS_TYPE_NONE; + if (sd->s_parent) + type = sysfs_ns_type(sd->s_parent); + if (type && (sysfs_info(dentry->d_sb)->ns[type] != sd->s_ns)) + goto out_bad; + mutex_unlock(&sysfs_mutex); out_valid: return 1; -- cgit v1.2.3 From 17f79be93d95bb0e46bd08681ec9c9e601869c15 Mon Sep 17 00:00:00 2001 From: Andrew Morton Date: Mon, 9 Jul 2012 16:13:36 -0700 Subject: sysfs: fail dentry revalidation after namespace change fix don't assume that KOBJ_NS_TYPE_NONE==0. Also save a test-n-branch. Cc: Eric W. Biederman Cc: Glauber Costa Cc: Tejun Heo Signed-off-by: Andrew Morton Acked-by: Serge E. Hallyn Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'fs/sysfs/dir.c') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index c0bf38a21ca..1cdfb53199a 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -329,10 +329,12 @@ static int sysfs_dentry_revalidate(struct dentry *dentry, struct nameidata *nd) /* The sysfs dirent has been moved to a different namespace */ type = KOBJ_NS_TYPE_NONE; - if (sd->s_parent) + if (sd->s_parent) { type = sysfs_ns_type(sd->s_parent); - if (type && (sysfs_info(dentry->d_sb)->ns[type] != sd->s_ns)) - goto out_bad; + if (type != KOBJ_NS_TYPE_NONE && + sysfs_info(dentry->d_sb)->ns[type] != sd->s_ns) + goto out_bad; + } mutex_unlock(&sysfs_mutex); out_valid: -- cgit v1.2.3 From 66081a72517a131430dcf986775f3268aafcb546 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Sat, 29 Sep 2012 22:23:19 +0200 Subject: sysfs: sysfs_pathname/sysfs_add_one: Use strlcat() instead of strcat() The warning check for duplicate sysfs entries can cause a buffer overflow when printing the warning, as strcat() doesn't check buffer sizes. Use strlcat() instead. Since strlcat() doesn't return a pointer to the passed buffer, unlike strcat(), I had to convert the nested concatenation in sysfs_add_one() to an admittedly more obscure comma operator construct, to avoid emitting code for the concatenation if CONFIG_BUG is disabled. Signed-off-by: Geert Uytterhoeven Cc: stable@vger.kernel.org Signed-off-by: Greg Kroah-Hartman --- fs/sysfs/dir.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'fs/sysfs/dir.c') diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c index 6b0bb00d4d2..2fbdff6be25 100644 --- a/fs/sysfs/dir.c +++ b/fs/sysfs/dir.c @@ -485,20 +485,18 @@ int __sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd) /** * sysfs_pathname - return full path to sysfs dirent * @sd: sysfs_dirent whose path we want - * @path: caller allocated buffer + * @path: caller allocated buffer of size PATH_MAX * * Gives the name "/" to the sysfs_root entry; any path returned * is relative to wherever sysfs is mounted. - * - * XXX: does no error checking on @path size */ static char *sysfs_pathname(struct sysfs_dirent *sd, char *path) { if (sd->s_parent) { sysfs_pathname(sd->s_parent, path); - strcat(path, "/"); + strlcat(path, "/", PATH_MAX); } - strcat(path, sd->s_name); + strlcat(path, sd->s_name, PATH_MAX); return path; } @@ -531,9 +529,11 @@ int sysfs_add_one(struct sysfs_addrm_cxt *acxt, struct sysfs_dirent *sd) char *path = kzalloc(PATH_MAX, GFP_KERNEL); WARN(1, KERN_WARNING "sysfs: cannot create duplicate filename '%s'\n", - (path == NULL) ? sd->s_name : - strcat(strcat(sysfs_pathname(acxt->parent_sd, path), "/"), - sd->s_name)); + (path == NULL) ? sd->s_name + : (sysfs_pathname(acxt->parent_sd, path), + strlcat(path, "/", PATH_MAX), + strlcat(path, sd->s_name, PATH_MAX), + path)); kfree(path); } -- cgit v1.2.3