Re: [patch 00/13] devtmpfs patches

[Kay Sievers <kay.sievers@vrfy.org>]

On Wed, 2009-05-13 at 10:35 -0400, Stephen Smalley wrote:
> > Maybe we could do the same credential swap in sysfs, and get rid of:
> > /**
> >  * lookup_one_noperm - bad hack for sysfs
> > 
> > Seems a bit odd to have a vfs function for a single filesystem, called
> > from a single location, and annotated as "do not use". Christoph added
> > the comment a while ago, so adding him to Cc:.
> 
> Yes, that makes sense to me as well - we didn't have the credentials
> infrastructure in place at the time that lookup_one_noperm was
> introduced, but switching the credentials around a normal lookup_one_len
> call should work now.

Something like this? It seems to work fine here, but I did not test it with SELinux.

Thanks,
Kay


From: Kay Sievers <kay.sievers@vrfy.org>
Subject: sysfs - switch noperm lookup_one_len() hack to credentials switch
Cc: Greg KH <greg@kroah.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: David Howells <dhowells@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>

Driver core actions may be requested by processes, which do not have the
proper permissions in a DAC and LSM/SELinux context to create entries in
sysfs. This replaces the vfs noperm hack with a switch to init_cred before
sysfs entries are created.

Signed-off-by: Kay Sievers <kay.sievers@vrfy.org>
---
 fs/namei.c            |   22 ----------------------
 fs/sysfs/dir.c        |    7 ++++++-
 include/linux/namei.h |    1 -
 3 files changed, 6 insertions(+), 24 deletions(-)

--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1260,28 +1260,6 @@ struct dentry *lookup_one_len(const char
 	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)
 {
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -21,6 +21,8 @@
 #include <linux/completion.h>
 #include <linux/mutex.h>
 #include <linux/slab.h>
+#include <linux/cred.h>
+#include <linux/init_task.h>
 #include "sysfs.h"
 
 DEFINE_MUTEX(sysfs_mutex);
@@ -103,6 +105,7 @@ struct dentry *sysfs_get_dentry(struct s
 
 	while (dentry->d_fsdata != sd) {
 		struct sysfs_dirent *cur;
+		const struct cred *curr_cred;
 		struct dentry *parent;
 
 		/* find the first ancestor which hasn't been looked up */
@@ -111,11 +114,13 @@ struct dentry *sysfs_get_dentry(struct s
 			cur = cur->s_parent;
 
 		/* look it up */
+		curr_cred = override_creds(&init_cred);
 		parent = dentry;
 		mutex_lock(&parent->d_inode->i_mutex);
-		dentry = lookup_one_noperm(cur->s_name, parent);
+		dentry = lookup_one_len(cur->s_name, parent, strlen(cur->s_name));
 		mutex_unlock(&parent->d_inode->i_mutex);
 		dput(parent);
+		revert_creds(curr_cred);
 
 		if (IS_ERR(dentry))
 			break;
--- a/include/linux/namei.h
+++ b/include/linux/namei.h
@@ -75,7 +75,6 @@ extern struct file *nameidata_to_filp(st
 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 vfsmount **, struct dentry **);
 extern int follow_up(struct vfsmount **, struct dentry **);