linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] Add vfsmount to vfs helper functions.
@ 2008-01-25 10:20 Kentaro Takeda
  2008-01-30 13:00 ` Al Viro
  0 siblings, 1 reply; 7+ messages in thread
From: Kentaro Takeda @ 2008-01-25 10:20 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: linux-security-module

In the LSM ml, we are discussing about
"how to know requested pathnames within LSM modules".

Currently, VFS helper functions don't pass "struct vfsmount" parameter.
Therefore, we cannot calculate requested pathnames within LSM modules
because LSM hooks can't know "struct vfsmount" parameter that corresponds with
"struct dentry" passed to VFS helper functions.

AppArmor is proposing a patch that appends "struct vfsmount" parameters to
VFS helper functions so that LSM modules (SELinux, AppArmor, TOMOYO) can
calculate requested pathnames.

The changes in include/linux/fs.h are shown below.
What do you think about these changes?

----- Start of changes -----
--- fs.h.orig
+++ fs.h
@@ -1070,13 +1070,13 @@
  */
 extern int vfs_permission(struct nameidata *, int);
 extern int vfs_create(struct inode *, struct dentry *, int, struct nameidata *);
-extern int vfs_mkdir(struct inode *, struct dentry *, int);
-extern int vfs_mknod(struct inode *, struct dentry *, int, dev_t);
-extern int vfs_symlink(struct inode *, struct dentry *, const char *, int);
-extern int vfs_link(struct dentry *, struct inode *, struct dentry *);
-extern int vfs_rmdir(struct inode *, struct dentry *);
-extern int vfs_unlink(struct inode *, struct dentry *);
-extern int vfs_rename(struct inode *, struct dentry *, struct inode *, struct dentry *);
+extern int vfs_mkdir(struct inode *, struct dentry *, struct vfsmount *, int);
+extern int vfs_mknod(struct inode *, struct dentry *, struct vfsmount *, int, dev_t);
+extern int vfs_symlink(struct inode *, struct dentry *, struct vfsmount *, const char *, int);
+extern int vfs_link(struct dentry *, struct vfsmount *, struct inode *, struct dentry *, struct vfsmount *);
+extern int vfs_rmdir(struct inode *, struct dentry *, struct vfsmount *);
+extern int vfs_unlink(struct inode *, struct dentry *, struct vfsmount *);
+extern int vfs_rename(struct inode *, struct dentry *, struct vfsmount *, struct inode *, struct dentry *, struct vfsmount *);
 
 /*
  * VFS dentry helper functions.
@@ -1538,8 +1538,8 @@
 
 /* fs/open.c */
 
-extern int do_truncate(struct dentry *, loff_t start, unsigned int time_attrs,
-		       struct file *filp);
+extern int do_truncate(struct dentry *, struct vfsmount *, loff_t start,
+		       unsigned int time_attrs, struct file *filp);
 extern long do_sys_open(int dfd, const char __user *filename, int flags,
 			int mode);
 extern struct file * dentry_open(struct dentry *, struct vfsmount *, int);
@@ -1695,7 +1695,7 @@
 #ifdef CONFIG_BLOCK
 extern sector_t bmap(struct inode *, sector_t);
 #endif
-extern int notify_change(struct dentry *, struct iattr *);
+extern int notify_change(struct dentry *, struct vfsmount *, struct iattr *);
 extern int permission(struct inode *, int, struct nameidata *);
 extern int generic_permission(struct inode *, int,
 		int (*check_acl)(struct inode *, int));
@@ -1757,9 +1757,9 @@
 extern void clear_inode(struct inode *);
 extern void destroy_inode(struct inode *);
 extern struct inode *new_inode(struct super_block *);
-extern int __remove_suid(struct dentry *, int);
+extern int __remove_suid(struct path *, int);
 extern int should_remove_suid(struct dentry *);
-extern int remove_suid(struct dentry *);
+extern int remove_suid(struct path *);
 
 extern void __insert_inode_hash(struct inode *, unsigned long hashval);
 extern void remove_inode_hash(struct inode *);
----- End of changes -----


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] Add vfsmount to vfs helper functions.
  2008-01-25 10:20 [RFC] Add vfsmount to vfs helper functions Kentaro Takeda
@ 2008-01-30 13:00 ` Al Viro
  2008-02-02 14:23   ` Tetsuo Handa
  0 siblings, 1 reply; 7+ messages in thread
From: Al Viro @ 2008-01-30 13:00 UTC (permalink / raw)
  To: Kentaro Takeda; +Cc: linux-fsdevel, linux-security-module

On Fri, Jan 25, 2008 at 07:20:56PM +0900, Kentaro Takeda wrote:
> In the LSM ml, we are discussing about
> "how to know requested pathnames within LSM modules".
> 
> Currently, VFS helper functions don't pass "struct vfsmount" parameter.
> Therefore, we cannot calculate requested pathnames within LSM modules
> because LSM hooks can't know "struct vfsmount" parameter that corresponds with
> "struct dentry" passed to VFS helper functions.
> 
> AppArmor is proposing a patch that appends "struct vfsmount" parameters to
> VFS helper functions so that LSM modules (SELinux, AppArmor, TOMOYO) can
> calculate requested pathnames.
> 
> The changes in include/linux/fs.h are shown below.
> What do you think about these changes?

That they are bloody *wrong*.  You have not addressed any of the objections
that had been posted too many times to repeat.  Damn it, you've not even
bothered to deal with the specific obvious stupidity with vfs_rename() -
just reposted the dreck and devil take all feedback.  Wonderful...

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] Add vfsmount to vfs helper functions.
  2008-01-30 13:00 ` Al Viro
@ 2008-02-02 14:23   ` Tetsuo Handa
  2008-02-17  9:00     ` Tetsuo Handa
  0 siblings, 1 reply; 7+ messages in thread
From: Tetsuo Handa @ 2008-02-02 14:23 UTC (permalink / raw)
  To: viro; +Cc: linux-fsdevel, linux-security-module

Hello.

Al Viro wrote:
> On Fri, Jan 25, 2008 at 07:20:56PM +0900, Kentaro Takeda wrote:
> > In the LSM ml, we are discussing about
> > "how to know requested pathnames within LSM modules".
> > 
> > Currently, VFS helper functions don't pass "struct vfsmount" parameter.
> > Therefore, we cannot calculate requested pathnames within LSM modules
> > because LSM hooks can't know "struct vfsmount" parameter that corresponds with
> > "struct dentry" passed to VFS helper functions.
> > 
> > AppArmor is proposing a patch that appends "struct vfsmount" parameters to
> > VFS helper functions so that LSM modules (SELinux, AppArmor, TOMOYO) can
> > calculate requested pathnames.
> > 
> > The changes in include/linux/fs.h are shown below.
> > What do you think about these changes?
> 
> That they are bloody *wrong*.  You have not addressed any of the objections
> that had been posted too many times to repeat.  Damn it, you've not even
> bothered to deal with the specific obvious stupidity with vfs_rename() -
> just reposted the dreck and devil take all feedback.  Wonderful...
> 
Thank you for your opinion.
Which one is closer to your opinion?

(1) I can never accept LSM modules from accessing
    pathname of requested "struct dentry".

(2) I accept LSM modules from accessing
    pathname of requested "struct dentry",
    but the approach is too ugly to accept.

If your opinion is (1), I have no idea to propose.

If your opinion is (2), there are several approaches.

(a) Add "struct vfsmount" to VFS related functions.

    This is what AppArmor proposes and TOMOYO 2.x can accept.
    http://lkml.org/lkml/2007/10/26/87

(b) Make "struct vfsmount" retrievable via "struct task_struct".

    This is what TOMOYO 2.x proposes. "struct task_struct" holds
    pointer to "struct vfsmount" which is pushed on the stack.
    http://lkml.org/lkml/2007/11/16/185

    Advantage is that we don't need to add "struct vfsmount" parameter to VFS functions.
    Disadvantage is that we have to add lots of VFS wrapper functions.

(c) Add new hooks.

    This is what TOMOYO 1.x is doing.
    Introduce a function which performs DAC checks (e.g. pre_vfs_link() at
    http://tomoyo.sourceforge.jp/cgi-bin/lxr/source/fs/namei.c?v=linux-2.6.24-tomoyo-1.5.3#L2421 )
    and a function which performs MAC checks (e.g. CheckDoubleWritePermission()).

    Advantage is that we don't need to add "struct vfsmount" parameter to VFS functions.
    Disadvantage is that we have to add lots of hooks and DAC permission checks are performed twice.

(d) Calculate pathname while doing name resolution (i.e. __link_path_walk() at fs/namei.c)
    and keep the result in "struct dentry" (or may be "struct file").

    This approach would yield the most accurate result because
    it doesn't access "struct dentry"->d_parent element.
    But I think it is a too large performance hit because it may entail
    memory allocation whenever one directory component is resolved and
    it is a memory consuming thing because pathnames stored in most "struct dentry" instances
    are not used by MAC checks.

    Saving pathname of name resolution time might help handling lazy unmounting problem.
    But is it desirable to reuse pathname of name resolution time even if the location of
    a file changed after the name resolution has completed?

(e) Calculate pathname inside LSM modules.

    This is not usable because this causes "namespace_sem" deadlock.
    http://lkml.org/lkml/2007/11/5/388

... and more possible approaches ?

Regards.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] Add vfsmount to vfs helper functions.
  2008-02-02 14:23   ` Tetsuo Handa
@ 2008-02-17  9:00     ` Tetsuo Handa
  2008-02-17 23:38       ` Al Viro
  0 siblings, 1 reply; 7+ messages in thread
From: Tetsuo Handa @ 2008-02-17  9:00 UTC (permalink / raw)
  To: viro; +Cc: linux-fsdevel, linux-security-module

Hello.

This is "(c) Add new hooks." approach I proposed at
http://www.mail-archive.com/linux-fsdevel@vger.kernel.org/msg11536.html .

Although this is an incomplete patch,
I want to know whether you can tolerate this approach or not.

If you cannot tolerate this approach, may be we need to consider
implementing "(d) Calculate pathname while doing name resolution" approach.

Regards.
----------
Subject: Call LSM functions outside VFS helper functions.

This patch allows LSM to check permission using "struct vfsmount"
without passing "struct vfsmount" to VFS helper functions.
There is a side effect that conventional permission checks are done twice
because I want to do DAC checks before MAC checks.

Signed-off-by:   Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 fs/namei.c               |  176 ++++++++++++++++++++++++++++++++++++++---------
 fs/open.c                |   11 ++
 include/linux/fs.h       |    1 
 include/linux/security.h |   63 ++++++++++++++++
 net/unix/af_unix.c       |    9 ++
 security/dummy.c         |   17 ++++
 security/security.c      |   15 ++++
 7 files changed, 254 insertions(+), 38 deletions(-)

--- linux-2.6.25-rc2-mm1.orig/fs/namei.c
+++ linux-2.6.25-rc2-mm1/fs/namei.c
@@ -2015,18 +2015,24 @@ fail:
 }
 EXPORT_SYMBOL_GPL(lookup_create);
 
-int vfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
+int pre_vfs_mknod(struct inode *dir, struct dentry *dentry, int mode)
 {
 	int error = may_create(dir, dentry, NULL);
-
 	if (error)
 		return error;
-
 	if ((S_ISCHR(mode) || S_ISBLK(mode)) && !capable(CAP_MKNOD))
 		return -EPERM;
-
 	if (!dir->i_op || !dir->i_op->mknod)
 		return -EPERM;
+	return 0;
+}
+EXPORT_SYMBOL(pre_vfs_mknod);
+
+int vfs_mknod(struct inode *dir, struct dentry *dentry, int mode, dev_t dev)
+{
+	int error = pre_vfs_mknod(dir, dentry, mode);
+	if (error)
+		return error;
 
 	error = security_inode_mknod(dir, dentry, mode, dev);
 	if (error)
@@ -2087,14 +2093,39 @@ asmlinkage long sys_mknodat(int dfd, con
 	if (error)
 		goto out_dput;
 	switch (mode & S_IFMT) {
+		int type;
 		case 0: case S_IFREG:
 			error = vfs_create(nd.path.dentry->d_inode,dentry,mode,&nd);
 			break;
 		case S_IFCHR: case S_IFBLK:
+			error = pre_vfs_mknod(nd.path.dentry->d_inode, dentry,
+					      mode);
+			if (error)
+				break;
+			if (S_ISCHR(mode))
+				type = TYPE_MKCHAR_ACL;
+			else
+				type = TYPE_MKBLOCK_ACL;
+			error = security_singlepath_permission(type, dentry,
+							       nd.path.mnt);
+			if (error)
+				break;
 			error = vfs_mknod(nd.path.dentry->d_inode,dentry,mode,
 					new_decode_dev(dev));
 			break;
 		case S_IFIFO: case S_IFSOCK:
+			error = pre_vfs_mknod(nd.path.dentry->d_inode, dentry,
+					      mode);
+			if (error)
+				break;
+			if (S_ISFIFO(mode))
+				type = TYPE_MKFIFO_ACL;
+			else
+				type = TYPE_MKSOCK_ACL;
+			error = security_singlepath_permission(type, dentry,
+							       nd.path.mnt);
+			if (error)
+				break;
 			error = vfs_mknod(nd.path.dentry->d_inode,dentry,mode,0);
 			break;
 	}
@@ -2115,15 +2146,21 @@ asmlinkage long sys_mknod(const char __u
 	return sys_mknodat(AT_FDCWD, filename, mode, dev);
 }
 
-int vfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
+static inline int pre_vfs_mkdir(struct inode *dir, struct dentry *dentry)
 {
 	int error = may_create(dir, dentry, NULL);
-
 	if (error)
 		return error;
-
 	if (!dir->i_op || !dir->i_op->mkdir)
 		return -EPERM;
+	return 0;
+}
+
+int vfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
+{
+	int error = pre_vfs_mkdir(dir, dentry);
+	if (error)
+		return error;
 
 	mode &= (S_IRWXUGO|S_ISVTX);
 	error = security_inode_mkdir(dir, dentry, mode);
@@ -2162,7 +2199,12 @@ asmlinkage long sys_mkdirat(int dfd, con
 	error = mnt_want_write(nd.path.mnt);
 	if (error)
 		goto out_dput;
-	error = vfs_mkdir(nd.path.dentry->d_inode, dentry, mode);
+	error = pre_vfs_mkdir(nd.path.dentry->d_inode, dentry);
+	if (!error)
+		error = security_singlepath_permission(TYPE_MKDIR_ACL, dentry,
+						       nd.path.mnt);
+	if (!error)
+		error = vfs_mkdir(nd.path.dentry->d_inode, dentry, mode);
 	mnt_drop_write(nd.path.mnt);
 out_dput:
 	dput(dentry);
@@ -2207,15 +2249,21 @@ void dentry_unhash(struct dentry *dentry
 	spin_unlock(&dcache_lock);
 }
 
-int vfs_rmdir(struct inode *dir, struct dentry *dentry)
+static inline int pre_vfs_rmdir(struct inode *dir, struct dentry *dentry)
 {
 	int error = may_delete(dir, dentry, 1);
-
 	if (error)
 		return error;
-
 	if (!dir->i_op || !dir->i_op->rmdir)
 		return -EPERM;
+	return 0;
+}
+
+int vfs_rmdir(struct inode *dir, struct dentry *dentry)
+{
+	int error = pre_vfs_rmdir(dir, dentry);
+	if (error)
+		return error;
 
 	DQUOT_INIT(dir);
 
@@ -2274,7 +2322,12 @@ static long do_rmdir(int dfd, const char
 	error = mnt_want_write(nd.path.mnt);
 	if (error)
 		goto exit3;
-	error = vfs_rmdir(nd.path.dentry->d_inode, dentry);
+	error = pre_vfs_rmdir(nd.path.dentry->d_inode, dentry);
+	if (!error)
+		error = security_singlepath_permission(TYPE_RMDIR_ACL, dentry,
+						       nd.path.mnt);
+	if (!error)
+		error = vfs_rmdir(nd.path.dentry->d_inode, dentry);
 	mnt_drop_write(nd.path.mnt);
 exit3:
 	dput(dentry);
@@ -2292,15 +2345,21 @@ asmlinkage long sys_rmdir(const char __u
 	return do_rmdir(AT_FDCWD, pathname);
 }
 
-int vfs_unlink(struct inode *dir, struct dentry *dentry)
+static inline int pre_vfs_unlink(struct inode *dir, struct dentry *dentry)
 {
 	int error = may_delete(dir, dentry, 0);
-
 	if (error)
 		return error;
-
 	if (!dir->i_op || !dir->i_op->unlink)
 		return -EPERM;
+	return 0;
+}
+
+int vfs_unlink(struct inode *dir, struct dentry *dentry)
+{
+	int error = pre_vfs_unlink(dir, dentry);
+	if (error)
+		return error;
 
 	DQUOT_INIT(dir);
 
@@ -2360,7 +2419,13 @@ static long do_unlinkat(int dfd, const c
 		error = mnt_want_write(nd.path.mnt);
 		if (error)
 			goto exit2;
-		error = vfs_unlink(nd.path.dentry->d_inode, dentry);
+		error = pre_vfs_unlink(nd.path.dentry->d_inode, dentry);
+		if (!error)
+			error = security_singlepath_permission(TYPE_UNLINK_ACL,
+							       dentry,
+							       nd.path.mnt);
+		if (!error)
+			error = vfs_unlink(nd.path.dentry->d_inode, dentry);
 		mnt_drop_write(nd.path.mnt);
 	exit2:
 		dput(dentry);
@@ -2396,15 +2461,22 @@ asmlinkage long sys_unlink(const char __
 	return do_unlinkat(AT_FDCWD, pathname);
 }
 
-int vfs_symlink(struct inode *dir, struct dentry *dentry, const char *oldname, int mode)
+static inline int pre_vfs_symlink(struct inode *dir, struct dentry *dentry)
 {
 	int error = may_create(dir, dentry, NULL);
-
 	if (error)
 		return error;
-
 	if (!dir->i_op || !dir->i_op->symlink)
 		return -EPERM;
+	return 0;
+}
+
+int vfs_symlink(struct inode *dir, struct dentry *dentry, const char *oldname,
+		int mode)
+{
+	int error = pre_vfs_symlink(dir, dentry);
+	if (error)
+		return error;
 
 	error = security_inode_symlink(dir, dentry, oldname);
 	if (error)
@@ -2445,7 +2517,13 @@ asmlinkage long sys_symlinkat(const char
 	error = mnt_want_write(nd.path.mnt);
 	if (error)
 		goto out_dput;
-	error = vfs_symlink(nd.path.dentry->d_inode, dentry, from, S_IALLUGO);
+	error = pre_vfs_symlink(nd.path.dentry->d_inode, dentry);
+	if (!error)
+		error = security_singlepath_permission(TYPE_SYMLINK_ACL,
+						       dentry, nd.path.mnt);
+	if (!error)
+		error = vfs_symlink(nd.path.dentry->d_inode, dentry, from,
+				    S_IALLUGO);
 	mnt_drop_write(nd.path.mnt);
 out_dput:
 	dput(dentry);
@@ -2464,21 +2542,18 @@ asmlinkage long sys_symlink(const char _
 	return sys_symlinkat(oldname, AT_FDCWD, newname);
 }
 
-int vfs_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_dentry)
+static inline int pre_vfs_link(struct dentry *old_dentry, struct inode *dir,
+			       struct dentry *new_dentry)
 {
 	struct inode *inode = old_dentry->d_inode;
 	int error;
-
 	if (!inode)
 		return -ENOENT;
-
 	error = may_create(dir, new_dentry, NULL);
 	if (error)
 		return error;
-
 	if (dir->i_sb != inode->i_sb)
 		return -EXDEV;
-
 	/*
 	 * A link to an append-only or immutable file cannot be created.
 	 */
@@ -2488,6 +2563,15 @@ int vfs_link(struct dentry *old_dentry, 
 		return -EPERM;
 	if (S_ISDIR(old_dentry->d_inode->i_mode))
 		return -EPERM;
+	return 0;
+}
+
+int vfs_link(struct dentry *old_dentry, struct inode *dir,
+	     struct dentry *new_dentry)
+{
+	int error = pre_vfs_link(old_dentry, dir, new_dentry);
+	if (error)
+		return error;
 
 	error = security_inode_link(old_dentry, dir, new_dentry);
 	if (error)
@@ -2545,7 +2629,16 @@ asmlinkage long sys_linkat(int olddfd, c
 	error = mnt_want_write(nd.path.mnt);
 	if (error)
 		goto out_dput;
-	error = vfs_link(old_nd.path.dentry, nd.path.dentry->d_inode, new_dentry);
+	error = pre_vfs_link(old_nd.path.dentry, nd.path.dentry->d_inode,
+			     new_dentry);
+	if (!error)
+		error = security_doublepath_permission(TYPE_LINK_ACL,
+						       old_nd.path.dentry,
+						       nd.path.dentry,
+						       old_nd.path.mnt);
+	if (!error)
+		error = vfs_link(old_nd.path.dentry, nd.path.dentry->d_inode,
+				 new_dentry);
 	mnt_drop_write(nd.path.mnt);
 out_dput:
 	dput(new_dentry);
@@ -2669,20 +2762,18 @@ static int vfs_rename_other(struct inode
 	return error;
 }
 
-int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
-	       struct inode *new_dir, struct dentry *new_dentry)
+static inline int pre_vfs_rename(struct inode *old_dir,
+				 struct dentry *old_dentry,
+				 struct inode *new_dir,
+				 struct dentry *new_dentry)
 {
 	int error;
 	int is_dir = S_ISDIR(old_dentry->d_inode->i_mode);
-	const char *old_name;
-
 	if (old_dentry->d_inode == new_dentry->d_inode)
  		return 0;
- 
 	error = may_delete(old_dir, old_dentry, is_dir);
 	if (error)
 		return error;
-
 	if (!new_dentry->d_inode)
 		error = may_create(new_dir, new_dentry, NULL);
 	else
@@ -2692,6 +2783,17 @@ int vfs_rename(struct inode *old_dir, st
 
 	if (!old_dir->i_op || !old_dir->i_op->rename)
 		return -EPERM;
+	return 0;
+}
+
+int vfs_rename(struct inode *old_dir, struct dentry *old_dentry,
+	       struct inode *new_dir, struct dentry *new_dentry)
+{
+	int error = pre_vfs_rename(old_dir, old_dentry, new_dir, new_dentry);
+	int is_dir = S_ISDIR(old_dentry->d_inode->i_mode);
+	const char *old_name;
+	if (error)
+		return error;
 
 	DQUOT_INIT(old_dir);
 	DQUOT_INIT(new_dir);
@@ -2776,7 +2878,15 @@ static int do_rename(int olddfd, const c
 	error = mnt_want_write(oldnd.path.mnt);
 	if (error)
 		goto exit5;
-	error = vfs_rename(old_dir->d_inode, old_dentry,
+	error = pre_vfs_rename(old_dir->d_inode, old_dentry, new_dir->d_inode,
+			       new_dentry);
+	if (!error)
+		error = security_doublepath_permission(TYPE_RENAME_ACL,
+						       oldnd.path.dentry,
+						       newnd.path.dentry,
+						       oldnd.path.mnt);
+	if (!error)
+		error = vfs_rename(old_dir->d_inode, old_dentry,
 				   new_dir->d_inode, new_dentry);
 	mnt_drop_write(oldnd.path.mnt);
 exit5:
--- linux-2.6.25-rc2-mm1.orig/include/linux/security.h
+++ linux-2.6.25-rc2-mm1/include/linux/security.h
@@ -112,6 +112,29 @@ struct request_sock;
 #define LSM_UNSAFE_PTRACE	2
 #define LSM_UNSAFE_PTRACE_CAP	4
 
+
+/* singlepath_permission() and doublepath_permission() */
+#define TYPE_READ_WRITE_ACL        0 /* open for reading/writing */
+#define TYPE_EXECUTE_ACL           1 /* execute                  */
+#define TYPE_READ_ACL              2 /* open for reading         */
+#define TYPE_WRITE_ACL             3 /* open for writing         */
+#define TYPE_CREATE_ACL            4 /* create                   */
+#define TYPE_UNLINK_ACL            5 /* unlink                   */
+#define TYPE_MKDIR_ACL             6 /* mkdir                    */
+#define TYPE_RMDIR_ACL             7 /* rmdir                    */
+#define TYPE_MKFIFO_ACL            8 /* mknod(S_IFIFO)           */
+#define TYPE_MKSOCK_ACL            9 /* mknod(S_IFSOCK)          */
+#define TYPE_MKBLOCK_ACL          10 /* mknod(S_IFBLK)           */
+#define TYPE_MKCHAR_ACL           11 /* mknod(S_IFCHR)           */
+#define TYPE_TRUNCATE_ACL         12 /* truncate                 */
+#define TYPE_SYMLINK_ACL          13 /* symlink                  */
+#define TYPE_REWRITE_ACL          14 /* open for overwriting     */
+#define MAX_SINGLE_PATH_OPERATION 15
+
+#define TYPE_LINK_ACL             0  /* link                     */
+#define TYPE_RENAME_ACL           1  /* rename                   */
+#define MAX_DOUBLE_PATH_OPERATION 2
+
 #ifdef CONFIG_SECURITY
 
 /**
@@ -307,6 +330,19 @@ struct request_sock;
  *	Returns 0 if @name and @value have been successfully set,
  *		-EOPNOTSUPP if no security attribute is needed, or
  *		-ENOMEM on memory allocation failure.
+ * @singlepath_permission:
+ *      Check permission to operations that involve one pathname.
+ *      @operation contains type of operation (e.g. mknod, mkdir, symlink).
+ *      @dentry contains the dentry structure for the file.
+ *      @mnt contains the vfsmount structure for the file.
+ *	Return 0 if permission is granted.
+ * @doublepath_permission:
+ *      Check permission to operations that involve two pathnames.
+ *      @operation contains type of operation (either link or rename).
+ *	@old_dentry contains the dentry structure of the old file.
+ *	@new_dentry contains the dentry structure of the new file.
+ *      @mnt contains the vfsmount structure for the old file.
+ *	Return 0 if permission is granted.
  * @inode_create:
  *	Check permission to create a regular file.
  *	@dir contains inode structure of the parent of the new file.
@@ -1279,6 +1315,11 @@ struct security_operations {
 	void (*inode_free_security) (struct inode *inode);
 	int (*inode_init_security) (struct inode *inode, struct inode *dir,
 				    char **name, void **value, size_t *len);
+	int (*singlepath_permission) (int operation, struct dentry *dentry,
+				      struct vfsmount *mnt);
+	int (*doublepath_permission) (int operation, struct dentry *old_dentry,
+				      struct dentry *new_dentry,
+				      struct vfsmount *mnt);
 	int (*inode_create) (struct inode *dir,
 	                     struct dentry *dentry, int mode);
 	int (*inode_link) (struct dentry *old_dentry,
@@ -1542,6 +1583,11 @@ int security_inode_alloc(struct inode *i
 void security_inode_free(struct inode *inode);
 int security_inode_init_security(struct inode *inode, struct inode *dir,
 				  char **name, void **value, size_t *len);
+int security_singlepath_permission(int operation, struct dentry *dentry,
+				   struct vfsmount *mnt);
+int security_doublepath_permission(int operation, struct dentry *old_dentry,
+				   struct dentry *new_dentry,
+				   struct vfsmount *mnt);
 int security_inode_create(struct inode *dir, struct dentry *dentry, int mode);
 int security_inode_link(struct dentry *old_dentry, struct inode *dir,
 			 struct dentry *new_dentry);
@@ -1847,7 +1893,22 @@ static inline int security_inode_init_se
 {
 	return -EOPNOTSUPP;
 }
-	
+
+static inline int security_singlepath_permission(int operation,
+						 struct dentry *dentry,
+						 struct vfsmount *mnt)
+{
+	return 0;
+}
+
+static inline int security_doublepath_permission(int operation,
+						 struct dentry *old_dentry,
+						 struct dentry *new_dentry,
+						 struct vfsmount *mnt)
+{
+	return 0;
+}
+
 static inline int security_inode_create (struct inode *dir,
 					 struct dentry *dentry,
 					 int mode)
--- linux-2.6.25-rc2-mm1.orig/security/dummy.c
+++ linux-2.6.25-rc2-mm1/security/dummy.c
@@ -284,6 +284,21 @@ static int dummy_inode_init_security (st
 	return -EOPNOTSUPP;
 }
 
+static inline int dummy_singlepath_permission(int operation,
+					      struct dentry *dentry,
+					      struct vfsmount *mnt)
+{
+	return 0;
+}
+
+static inline int dummy_doublepath_permission(int operation,
+					      struct dentry *old_dentry,
+					      struct dentry *new_dentry,
+					      struct vfsmount *mnt)
+{
+	return 0;
+}
+
 static int dummy_inode_create (struct inode *inode, struct dentry *dentry,
 			       int mask)
 {
@@ -1038,6 +1053,8 @@ void security_fixup_ops (struct security
 	set_to_dummy_if_null(ops, inode_alloc_security);
 	set_to_dummy_if_null(ops, inode_free_security);
 	set_to_dummy_if_null(ops, inode_init_security);
+	set_to_dummy_if_null(ops, singlepath_permission);
+	set_to_dummy_if_null(ops, doublepath_permission);
 	set_to_dummy_if_null(ops, inode_create);
 	set_to_dummy_if_null(ops, inode_link);
 	set_to_dummy_if_null(ops, inode_unlink);
--- linux-2.6.25-rc2-mm1.orig/security/security.c
+++ linux-2.6.25-rc2-mm1/security/security.c
@@ -345,6 +345,21 @@ int security_inode_init_security(struct 
 }
 EXPORT_SYMBOL(security_inode_init_security);
 
+int security_singlepath_permission(int operation, struct dentry *dentry,
+				   struct vfsmount *mnt)
+{
+	return security_ops->singlepath_permission(operation, dentry, mnt);
+}
+EXPORT_SYMBOL(security_singlepath_permission);
+
+int security_doublepath_permission(int operation, struct dentry *old_dentry,
+				   struct dentry *new_dentry,
+				   struct vfsmount *mnt)
+{
+	return security_ops->doublepath_permission(operation, old_dentry,
+						   new_dentry, mnt);
+}
+
 int security_inode_create(struct inode *dir, struct dentry *dentry, int mode)
 {
 	if (unlikely(IS_PRIVATE(dir)))
--- linux-2.6.25-rc2-mm1.orig/net/unix/af_unix.c
+++ linux-2.6.25-rc2-mm1/net/unix/af_unix.c
@@ -822,7 +822,14 @@ static int unix_bind(struct socket *sock
 		err = mnt_want_write(nd.path.mnt);
 		if (err)
 			goto out_mknod_dput;
-		err = vfs_mknod(nd.path.dentry->d_inode, dentry, mode, 0);
+		err = pre_vfs_mknod(nd.path.dentry->d_inode, dentry, mode);
+		if (!err)
+			err = security_singlepath_permission(TYPE_MKSOCK_ACL,
+							     dentry,
+							     nd.path.mnt);
+		if (!err)
+			err = vfs_mknod(nd.path.dentry->d_inode, dentry,
+					mode, 0);
 		mnt_drop_write(nd.path.mnt);
 		if (err)
 			goto out_mknod_dput;
--- linux-2.6.25-rc2-mm1.orig/include/linux/fs.h
+++ linux-2.6.25-rc2-mm1/include/linux/fs.h
@@ -1126,6 +1126,7 @@ extern void unlock_super(struct super_bl
 extern int vfs_permission(struct nameidata *, int);
 extern int vfs_create(struct inode *, struct dentry *, int, struct nameidata *);
 extern int vfs_mkdir(struct inode *, struct dentry *, int);
+extern int pre_vfs_mknod(struct inode *, struct dentry *, int);
 extern int vfs_mknod(struct inode *, struct dentry *, int, dev_t);
 extern int vfs_symlink(struct inode *, struct dentry *, const char *, int);
 extern int vfs_link(struct dentry *, struct inode *, struct dentry *);
--- linux-2.6.25-rc2-mm1.orig/fs/open.c
+++ linux-2.6.25-rc2-mm1/fs/open.c
@@ -267,8 +267,10 @@ static long do_sys_truncate(const char _
 	error = break_lease(inode, FMODE_WRITE);
 	if (error)
 		goto put_write_and_out;
-
-	error = locks_verify_truncate(inode, NULL, length);
+	error = security_singlepath_permission(TYPE_TRUNCATE_ACL,
+					       nd.path.dentry, nd.path.mnt);
+	if (!error)
+		error = locks_verify_truncate(inode, NULL, length);
 	if (!error) {
 		DQUOT_INIT(inode);
 		error = do_truncate(nd.path.dentry, length, 0, NULL);
@@ -324,7 +326,10 @@ static long do_sys_ftruncate(unsigned in
 	if (IS_APPEND(inode))
 		goto out_putf;
 
-	error = locks_verify_truncate(inode, file, length);
+	error = security_singlepath_permission(TYPE_TRUNCATE_ACL, dentry,
+					       file->f_path.mnt);
+	if (!error)
+		error = locks_verify_truncate(inode, file, length);
 	if (!error)
 		error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, file);
 out_putf:

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] Add vfsmount to vfs helper functions.
  2008-02-17  9:00     ` Tetsuo Handa
@ 2008-02-17 23:38       ` Al Viro
  2008-02-18  0:03         ` Tetsuo Handa
  0 siblings, 1 reply; 7+ messages in thread
From: Al Viro @ 2008-02-17 23:38 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: linux-fsdevel, linux-security-module

On Sun, Feb 17, 2008 at 06:00:30PM +0900, Tetsuo Handa wrote:
> Hello.
> 
> This is "(c) Add new hooks." approach I proposed at
> http://www.mail-archive.com/linux-fsdevel@vger.kernel.org/msg11536.html .
> 
> Although this is an incomplete patch,
> I want to know whether you can tolerate this approach or not.
> 
> If you cannot tolerate this approach, may be we need to consider
> implementing "(d) Calculate pathname while doing name resolution" approach.
> 

No printable comments, except for that:

(e) why don't you guys move the Linus' Serious Mistake to _callers_ of
vfs_mknod() and its ilk?

Which obviously solves all problems with having vfsmount.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] Add vfsmount to vfs helper functions.
  2008-02-17 23:38       ` Al Viro
@ 2008-02-18  0:03         ` Tetsuo Handa
  2008-02-18  0:40           ` Al Viro
  0 siblings, 1 reply; 7+ messages in thread
From: Tetsuo Handa @ 2008-02-18  0:03 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-fsdevel, linux-security-module

Hello.

> No printable comments, except for that:
> 
> (e) why don't you guys move the Linus' Serious Mistake to _callers_ of
> vfs_mknod() and its ilk?
> 
> Which obviously solves all problems with having vfsmount.

Excuse me. I didn't understand what "the Linus' Serious Mistake to
_callers_ of vfs_mknod()" is. Could you give me some URLs or hints?

Regards.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] Add vfsmount to vfs helper functions.
  2008-02-18  0:03         ` Tetsuo Handa
@ 2008-02-18  0:40           ` Al Viro
  0 siblings, 0 replies; 7+ messages in thread
From: Al Viro @ 2008-02-18  0:40 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: linux-fsdevel, linux-security-module

On Mon, Feb 18, 2008 at 09:03:51AM +0900, Tetsuo Handa wrote:
> Hello.
> 
> > No printable comments, except for that:
> > 
> > (e) why don't you guys move the Linus' Serious Mistake to _callers_ of
> > vfs_mknod() and its ilk?
> > 
> > Which obviously solves all problems with having vfsmount.
> 
> Excuse me. I didn't understand what "the Linus' Serious Mistake to
> _callers_ of vfs_mknod()" is. Could you give me some URLs or hints?

Linus' Serious Mistake: LSM.

Moving that junk to callers for vfs_mknod(): should be obvious; remove
the calls of security_whatever() from vfs_...() and add them in places
that call the functions in question.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2008-02-18  0:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-01-25 10:20 [RFC] Add vfsmount to vfs helper functions Kentaro Takeda
2008-01-30 13:00 ` Al Viro
2008-02-02 14:23   ` Tetsuo Handa
2008-02-17  9:00     ` Tetsuo Handa
2008-02-17 23:38       ` Al Viro
2008-02-18  0:03         ` Tetsuo Handa
2008-02-18  0:40           ` Al Viro

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).