linux-kernel - Re: [patch 00/13] devtmpfs patches

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1242233101.2893.7.camel@poy>
Date:	Wed, 13 May 2009 18:45:01 +0200
From:	Kay Sievers <kay.sievers@...y.org>
To:	Stephen Smalley <sds@...ho.nsa.gov>
Cc:	David Howells <dhowells@...hat.com>,
	"David P. Quigley" <dpquigl@...ho.nsa.gov>,
	Greg KH <greg@...ah.com>, linux-kernel@...r.kernel.org,
	Greg KH <gregkh@...e.de>, Jan Blunck <jblunck@...e.de>,
	James Morris <jmorris@...ei.org>,
	Eric Paris <eparis@...isplace.org>,
	Christoph Hellwig <hch@...radead.org>
Subject: Re: [patch 00/13] devtmpfs patches

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@...y.org>
Subject: sysfs - switch noperm lookup_one_len() hack to credentials switch
Cc: Greg KH <greg@...ah.com>
Cc: Stephen Smalley <sds@...ho.nsa.gov>
Cc: David Howells <dhowells@...hat.com>
Cc: Christoph Hellwig <hch@...radead.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@...y.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 **);


--
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/