* [PATCH] Introduce new LSM hooks where vfsmount is available.
[not found] ` <48C877DE.4040402@nttdata.co.jp>
@ 2008-09-17 2:16 ` Kentaro Takeda
2008-09-17 4:00 ` Casey Schaufler
2008-09-23 6:06 ` James Morris
0 siblings, 2 replies; 16+ messages in thread
From: Kentaro Takeda @ 2008-09-17 2:16 UTC (permalink / raw)
To: linux-security-module; +Cc: linux-fsdevel, linux-kernel, Toshiharu Harada
TOMOYO Linux needs method for calculating pathname in LSM module.
However, we have received comment from Al Viro, the vfs maintainer,
that adding vfsmount parameter to vfs helper functions (and LSM hooks)
is not preferable. We have asked some people (including Al), and we
came back to the most straightforward approach; adding new LSM hooks
where vfsmount is available.
The attached patch introduces several new LSM hooks TOMOYO Linux
needs. It has less impact to existing LSM module and no impact to vfs
helper functions. Please review it.
Regards,
---
Subject: Introduce new LSM hooks.
This patch allows LSM to check permission using "struct vfsmount"
without passing "struct vfsmount" to VFS helper functions.
Signed-off-by: Kentaro Takeda <takedakn@nttdata.co.jp>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Toshiharu Harada <haradats@nttdata.co.jp>
---
fs/namei.c | 37 ++++++++++++
fs/open.c | 5 +
include/linux/security.h | 135 +++++++++++++++++++++++++++++++++++++++++++++++
net/unix/af_unix.c | 4 +
security/capability.c | 53 ++++++++++++++++++
security/security.c | 63 +++++++++++++++++++++
6 files changed, 297 insertions(+)
--- linux-2.6.27-rc6.orig/fs/namei.c
+++ linux-2.6.27-rc6/fs/namei.c
@@ -1585,6 +1585,10 @@ int may_open(struct nameidata *nd, int a
* Refuse to truncate files with mandatory locks held on them.
*/
error = locks_verify_locked(inode);
+ if (!error)
+ error = security_path_truncate(&nd->path, 0,
+ ATTR_MTIME|ATTR_CTIME|ATTR_OPEN,
+ NULL);
if (!error) {
DQUOT_INIT(inode);
@@ -1615,7 +1619,11 @@ static int __open_namei_create(struct na
if (!IS_POSIXACL(dir->d_inode))
mode &= ~current->fs->umask;
+ error = security_path_mknod(&nd->path, path->dentry, mode, 0);
+ if (error)
+ goto out_unlock;
error = vfs_create(dir->d_inode, path->dentry, mode, nd);
+out_unlock:
mutex_unlock(&dir->d_inode->i_mutex);
dput(nd->path.dentry);
nd->path.dentry = path->dentry;
@@ -2018,6 +2026,9 @@ asmlinkage long sys_mknodat(int dfd, con
error = mnt_want_write(nd.path.mnt);
if (error)
goto out_dput;
+ error = security_path_mknod(&nd.path, dentry, mode, dev);
+ if (error)
+ goto out_drop_write;
switch (mode & S_IFMT) {
case 0: case S_IFREG:
error = vfs_create(nd.path.dentry->d_inode,dentry,mode,&nd);
@@ -2030,6 +2041,7 @@ asmlinkage long sys_mknodat(int dfd, con
error = vfs_mknod(nd.path.dentry->d_inode,dentry,mode,0);
break;
}
+out_drop_write:
mnt_drop_write(nd.path.mnt);
out_dput:
dput(dentry);
@@ -2089,7 +2101,11 @@ asmlinkage long sys_mkdirat(int dfd, con
error = mnt_want_write(nd.path.mnt);
if (error)
goto out_dput;
+ error = security_path_mkdir(&nd.path, dentry, mode);
+ if (error)
+ goto out_drop_write;
error = vfs_mkdir(nd.path.dentry->d_inode, dentry, mode);
+out_drop_write:
mnt_drop_write(nd.path.mnt);
out_dput:
dput(dentry);
@@ -2196,7 +2212,11 @@ static long do_rmdir(int dfd, const char
error = mnt_want_write(nd.path.mnt);
if (error)
goto exit3;
+ error = security_path_rmdir(&nd.path, dentry);
+ if (error)
+ goto exit4;
error = vfs_rmdir(nd.path.dentry->d_inode, dentry);
+exit4:
mnt_drop_write(nd.path.mnt);
exit3:
dput(dentry);
@@ -2278,7 +2298,11 @@ static long do_unlinkat(int dfd, const c
error = mnt_want_write(nd.path.mnt);
if (error)
goto exit2;
+ error = security_path_unlink(&nd.path, dentry);
+ if (error)
+ goto exit3;
error = vfs_unlink(nd.path.dentry->d_inode, dentry);
+exit3:
mnt_drop_write(nd.path.mnt);
exit2:
dput(dentry);
@@ -2359,7 +2383,11 @@ asmlinkage long sys_symlinkat(const char
error = mnt_want_write(nd.path.mnt);
if (error)
goto out_dput;
+ error = security_path_symlink(&nd.path, dentry, from);
+ if (error)
+ goto out_drop_write;
error = vfs_symlink(nd.path.dentry->d_inode, dentry, from);
+out_drop_write:
mnt_drop_write(nd.path.mnt);
out_dput:
dput(dentry);
@@ -2456,7 +2484,11 @@ asmlinkage long sys_linkat(int olddfd, c
error = mnt_want_write(nd.path.mnt);
if (error)
goto out_dput;
+ error = security_path_link(old_path.dentry, &nd.path, new_dentry);
+ if (error)
+ goto out_drop_write;
error = vfs_link(old_path.dentry, nd.path.dentry->d_inode, new_dentry);
+out_drop_write:
mnt_drop_write(nd.path.mnt);
out_dput:
dput(new_dentry);
@@ -2688,8 +2720,13 @@ asmlinkage long sys_renameat(int olddfd,
error = mnt_want_write(oldnd.path.mnt);
if (error)
goto exit5;
+ error = security_path_rename(&oldnd.path, old_dentry,
+ &newnd.path, new_dentry);
+ if (error)
+ goto exit6;
error = vfs_rename(old_dir->d_inode, old_dentry,
new_dir->d_inode, new_dentry);
+exit6:
mnt_drop_write(oldnd.path.mnt);
exit5:
dput(new_dentry);
--- linux-2.6.27-rc6.orig/fs/open.c
+++ linux-2.6.27-rc6/fs/open.c
@@ -272,6 +272,8 @@ static long do_sys_truncate(const char _
goto put_write_and_out;
error = locks_verify_truncate(inode, NULL, length);
+ if (!error)
+ error = security_path_truncate(&path, length, 0, NULL);
if (!error) {
DQUOT_INIT(inode);
error = do_truncate(path.dentry, length, 0, NULL);
@@ -329,6 +331,9 @@ static long do_sys_ftruncate(unsigned in
error = locks_verify_truncate(inode, file, length);
if (!error)
+ error = security_path_truncate(&file->f_path, length,
+ ATTR_MTIME|ATTR_CTIME, file);
+ if (!error)
error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, file);
out_putf:
fput(file);
--- linux-2.6.27-rc6.orig/include/linux/security.h
+++ linux-2.6.27-rc6/include/linux/security.h
@@ -345,17 +345,37 @@ static inline void security_free_mnt_opt
* @dir contains the inode structure of the parent directory of the new link.
* @new_dentry contains the dentry structure for the new link.
* Return 0 if permission is granted.
+ * @path_link:
+ * Check permission before creating a new hard link to a file.
+ * @old_dentry contains the dentry structure for an existing link
+ * to the file.
+ * @new_dir contains the path structure of the parent directory of
+ * the new link.
+ * @new_dentry contains the dentry structure for the new link.
+ * Return 0 if permission is granted.
* @inode_unlink:
* Check the permission to remove a hard link to a file.
* @dir contains the inode structure of parent directory of the file.
* @dentry contains the dentry structure for file to be unlinked.
* Return 0 if permission is granted.
+ * @path_unlink:
+ * Check the permission to remove a hard link to a file.
+ * @dir contains the path structure of parent directory of the file.
+ * @dentry contains the dentry structure for file to be unlinked.
+ * Return 0 if permission is granted.
* @inode_symlink:
* Check the permission to create a symbolic link to a file.
* @dir contains the inode structure of parent directory of the symbolic link.
* @dentry contains the dentry structure of the symbolic link.
* @old_name contains the pathname of file.
* Return 0 if permission is granted.
+ * @path_symlink:
+ * Check the permission to create a symbolic link to a file.
+ * @dir contains the path structure of parent directory of
+ * the symbolic link.
+ * @dentry contains the dentry structure of the symbolic link.
+ * @old_name contains the pathname of file.
+ * Return 0 if permission is granted.
* @inode_mkdir:
* Check permissions to create a new directory in the existing directory
* associated with inode strcture @dir.
@@ -363,11 +383,25 @@ static inline void security_free_mnt_opt
* @dentry contains the dentry structure of new directory.
* @mode contains the mode of new directory.
* Return 0 if permission is granted.
+ * @path_mkdir:
+ * Check permissions to create a new directory in the existing directory
+ * associated with path strcture @path.
+ * @dir containst the path structure of parent of the directory
+ * to be created.
+ * @dentry contains the dentry structure of new directory.
+ * @mode contains the mode of new directory.
+ * Return 0 if permission is granted.
* @inode_rmdir:
* Check the permission to remove a directory.
* @dir contains the inode structure of parent of the directory to be removed.
* @dentry contains the dentry structure of directory to be removed.
* Return 0 if permission is granted.
+ * @path_rmdir:
+ * Check the permission to remove a directory.
+ * @dir contains the path structure of parent of the directory to be
+ * removed.
+ * @dentry contains the dentry structure of directory to be removed.
+ * Return 0 if permission is granted.
* @inode_mknod:
* Check permissions when creating a special file (or a socket or a fifo
* file created via the mknod system call). Note that if mknod operation
@@ -378,6 +412,15 @@ static inline void security_free_mnt_opt
* @mode contains the mode of the new file.
* @dev contains the device number.
* Return 0 if permission is granted.
+ * @path_mknod:
+ * Check permissions when creating a file. Note that this hook is called
+ * even if mknod operation is being done for a regular file.
+ * @dir contains the path structure of parent of the new file.
+ * @dentry contains the dentry structure of the new file.
+ * @mode contains the mode of the new file.
+ * @dev contains the undecoded device number. Use new_decode_dev() to get
+ * the decoded device number.
+ * Return 0 if permission is granted.
* @inode_rename:
* Check for permission to rename a file or directory.
* @old_dir contains the inode structure for parent of the old link.
@@ -385,6 +428,13 @@ static inline void security_free_mnt_opt
* @new_dir contains the inode structure for parent of the new link.
* @new_dentry contains the dentry structure of the new link.
* Return 0 if permission is granted.
+ * @path_rename:
+ * Check for permission to rename a file or directory.
+ * @old_dir contains the path structure for parent of the old link.
+ * @old_dentry contains the dentry structure of the old link.
+ * @new_dir contains the path structure for parent of the new link.
+ * @new_dentry contains the dentry structure of the new link.
+ * Return 0 if permission is granted.
* @inode_readlink:
* Check the permission to read the symbolic link.
* @dentry contains the dentry structure for the file link.
@@ -413,6 +463,13 @@ static inline void security_free_mnt_opt
* @dentry contains the dentry structure for the file.
* @attr is the iattr structure containing the new file attributes.
* Return 0 if permission is granted.
+ * @path_truncate:
+ * Check permission before truncating a file.
+ * @path contains the path structure for the file.
+ * @length is the new length of the file.
+ * @time_attrs is the flags passed to do_truncate().
+ * @filp is the file structure (may be NULL).
+ * Return 0 if permission is granted.
* @inode_getattr:
* Check permission before obtaining file attributes.
* @mnt is the vfsmount where the dentry was looked up
@@ -1350,6 +1407,20 @@ struct security_operations {
struct super_block *newsb);
int (*sb_parse_opts_str) (char *options, struct security_mnt_opts *opts);
+ int (*path_unlink) (struct path *dir, struct dentry *dentry);
+ int (*path_mkdir) (struct path *dir, struct dentry *dentry, int mode);
+ int (*path_rmdir) (struct path *dir, struct dentry *dentry);
+ int (*path_mknod) (struct path *dir, struct dentry *dentry, int mode,
+ unsigned int dev);
+ int (*path_truncate) (struct path *path, loff_t length,
+ unsigned int time_attrs, struct file *filp);
+ int (*path_symlink) (struct path *dir, struct dentry *dentry,
+ const char *old_name);
+ int (*path_link) (struct dentry *old_dentry, struct path *new_dir,
+ struct dentry *new_dentry);
+ int (*path_rename) (struct path *old_dir, struct dentry *old_dentry,
+ struct path *new_dir, struct dentry *new_dentry);
+
int (*inode_alloc_security) (struct inode *inode);
void (*inode_free_security) (struct inode *inode);
int (*inode_init_security) (struct inode *inode, struct inode *dir,
@@ -1618,6 +1689,20 @@ void security_sb_clone_mnt_opts(const st
struct super_block *newsb);
int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
+int security_path_unlink(struct path *dir, struct dentry *dentry);
+int security_path_mkdir(struct path *dir, struct dentry *dentry, int mode);
+int security_path_rmdir(struct path *dir, struct dentry *dentry);
+int security_path_mknod(struct path *dir, struct dentry *dentry, int mode,
+ unsigned int dev);
+int security_path_truncate(struct path *path, loff_t length,
+ unsigned int time_attrs, struct file *filp);
+int security_path_symlink(struct path *dir, struct dentry *dentry,
+ const char *old_name);
+int security_path_link(struct dentry *old_dentry, struct path *new_dir,
+ struct dentry *new_dentry);
+int security_path_rename(struct path *old_dir, struct dentry *old_dentry,
+ struct path *new_dir, struct dentry *new_dentry);
+
int security_inode_alloc(struct inode *inode);
void security_inode_free(struct inode *inode);
int security_inode_init_security(struct inode *inode, struct inode *dir,
@@ -1949,6 +2034,56 @@ static inline int security_sb_parse_opts
return 0;
}
+static inline int security_path_unlink(struct path *dir, struct dentry *dentry)
+{
+ return 0;
+}
+
+static inline int security_path_mkdir(struct path *dir, struct dentry *dentry,
+ int mode)
+{
+ return 0;
+}
+
+static inline int security_path_rmdir(struct path *dir, struct dentry *dentry)
+{
+ return 0;
+}
+
+static inline int security_path_mknod(struct path *dir, struct dentry *dentry,
+ int mode, unsigned int dev)
+{
+ return 0;
+}
+
+static inline int security_path_truncate(struct path *path, loff_t length,
+ unsigned int time_attrs,
+ struct file *filp)
+{
+ return 0;
+}
+
+static inline int security_path_symlink(struct path *dir, struct dentry *dentry,
+ const char *old_name)
+{
+ return 0;
+}
+
+static inline int security_path_link(struct dentry *old_dentry,
+ struct path *new_dir,
+ struct dentry *new_dentry)
+{
+ return 0;
+}
+
+static inline int security_path_rename(struct path *old_dir,
+ struct dentry *old_dentry,
+ struct path *new_dir,
+ struct dentry *new_dentry)
+{
+ return 0;
+}
+
static inline int security_inode_alloc(struct inode *inode)
{
return 0;
--- linux-2.6.27-rc6.orig/net/unix/af_unix.c
+++ linux-2.6.27-rc6/net/unix/af_unix.c
@@ -827,7 +827,11 @@ static int unix_bind(struct socket *sock
err = mnt_want_write(nd.path.mnt);
if (err)
goto out_mknod_dput;
+ err = security_path_mknod(&nd.path, dentry, mode, 0);
+ if (err)
+ goto out_mknod_drop_write;
err = vfs_mknod(nd.path.dentry->d_inode, dentry, mode, 0);
+out_mknod_drop_write:
mnt_drop_write(nd.path.mnt);
if (err)
goto out_mknod_dput;
--- linux-2.6.27-rc6.orig/security/capability.c
+++ linux-2.6.27-rc6/security/capability.c
@@ -268,6 +268,51 @@ static void cap_inode_getsecid(const str
*secid = 0;
}
+static int cap_path_mknod(struct path *dir, struct dentry *dentry, int mode,
+ unsigned int dev)
+{
+ return 0;
+}
+
+static int cap_path_mkdir(struct path *dir, struct dentry *dentry, int mode)
+{
+ return 0;
+}
+
+static int cap_path_rmdir(struct path *dir, struct dentry *dentry)
+{
+ return 0;
+}
+
+static int cap_path_unlink(struct path *dir, struct dentry *dentry)
+{
+ return 0;
+}
+
+static int cap_path_symlink(struct path *dir, struct dentry *dentry,
+ const char *old_name)
+{
+ return 0;
+}
+
+static int cap_path_link(struct dentry *old_dentry, struct path *new_dir,
+ struct dentry *new_dentry)
+{
+ return 0;
+}
+
+static int cap_path_rename(struct path *old_path, struct dentry *old_dentry,
+ struct path *new_path, struct dentry *new_dentry)
+{
+ return 0;
+}
+
+static int cap_path_truncate(struct path *path, loff_t length,
+ unsigned int time_attrs, struct file *filp)
+{
+ return 0;
+}
+
static int cap_file_permission(struct file *file, int mask)
{
return 0;
@@ -877,6 +922,14 @@ void security_fixup_ops(struct security_
set_to_cap_if_null(ops, inode_setsecurity);
set_to_cap_if_null(ops, inode_listsecurity);
set_to_cap_if_null(ops, inode_getsecid);
+ set_to_cap_if_null(ops, path_mknod);
+ set_to_cap_if_null(ops, path_mkdir);
+ set_to_cap_if_null(ops, path_rmdir);
+ set_to_cap_if_null(ops, path_unlink);
+ set_to_cap_if_null(ops, path_symlink);
+ set_to_cap_if_null(ops, path_link);
+ set_to_cap_if_null(ops, path_rename);
+ set_to_cap_if_null(ops, path_truncate);
set_to_cap_if_null(ops, file_permission);
set_to_cap_if_null(ops, file_alloc_security);
set_to_cap_if_null(ops, file_free_security);
--- linux-2.6.27-rc6.orig/security/security.c
+++ linux-2.6.27-rc6/security/security.c
@@ -358,6 +358,69 @@ int security_inode_init_security(struct
}
EXPORT_SYMBOL(security_inode_init_security);
+int security_path_mknod(struct path *path, struct dentry *dentry, int mode,
+ unsigned int dev)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_mknod(path, dentry, mode, dev);
+}
+
+int security_path_mkdir(struct path *path, struct dentry *dentry, int mode)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_mkdir(path, dentry, mode);
+}
+
+int security_path_rmdir(struct path *path, struct dentry *dentry)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_rmdir(path, dentry);
+}
+
+int security_path_unlink(struct path *path, struct dentry *dentry)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_unlink(path, dentry);
+}
+
+int security_path_symlink(struct path *path, struct dentry *dentry,
+ const char *old_name)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_symlink(path, dentry, old_name);
+}
+
+int security_path_link(struct dentry *old_dentry, struct path *new_dir,
+ struct dentry *new_dentry)
+{
+ if (unlikely(IS_PRIVATE(old_dentry->d_inode)))
+ return 0;
+ return security_ops->path_link(old_dentry, new_dir, new_dentry);
+}
+
+int security_path_rename(struct path *old_dir, struct dentry *old_dentry,
+ struct path *new_dir, struct dentry *new_dentry)
+{
+ if (unlikely(IS_PRIVATE(old_dentry->d_inode) ||
+ (new_dentry->d_inode && IS_PRIVATE(new_dentry->d_inode))))
+ return 0;
+ return security_ops->path_rename(old_dir, old_dentry, new_dir,
+ new_dentry);
+}
+
+int security_path_truncate(struct path *path, loff_t length,
+ unsigned int time_attrs, struct file *filp)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_truncate(path, length, time_attrs, filp);
+}
+
int security_inode_create(struct inode *dir, struct dentry *dentry, int mode)
{
if (unlikely(IS_PRIVATE(dir)))
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Introduce new LSM hooks where vfsmount is available.
2008-09-17 2:16 ` [PATCH] Introduce " Kentaro Takeda
@ 2008-09-17 4:00 ` Casey Schaufler
2008-09-17 8:35 ` Kentaro Takeda
2008-09-17 17:46 ` david
2008-09-23 6:06 ` James Morris
1 sibling, 2 replies; 16+ messages in thread
From: Casey Schaufler @ 2008-09-17 4:00 UTC (permalink / raw)
To: Kentaro Takeda
Cc: linux-security-module, linux-fsdevel, linux-kernel,
Toshiharu Harada
Kentaro Takeda wrote:
> TOMOYO Linux needs method for calculating pathname in LSM module.
> However, we have received comment from Al Viro, the vfs maintainer,
> that adding vfsmount parameter to vfs helper functions (and LSM hooks)
> is not preferable. We have asked some people (including Al), and we
> came back to the most straightforward approach; adding new LSM hooks
> where vfsmount is available.
>
> The attached patch introduces several new LSM hooks TOMOYO Linux
> needs. It has less impact to existing LSM module and no impact to vfs
> helper functions. Please review it.
>
> Regards,
>
>
As always, the community will be eager to see the code that makes
use of these hooks.
> ---
> Subject: Introduce new LSM hooks.
>
> This patch allows LSM to check permission using "struct vfsmount"
> without passing "struct vfsmount" to VFS helper functions.
>
> Signed-off-by: Kentaro Takeda <takedakn@nttdata.co.jp>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Signed-off-by: Toshiharu Harada <haradats@nttdata.co.jp>
> ---
> fs/namei.c | 37 ++++++++++++
> fs/open.c | 5 +
> include/linux/security.h | 135 +++++++++++++++++++++++++++++++++++++++++++++++
> net/unix/af_unix.c | 4 +
> security/capability.c | 53 ++++++++++++++++++
> security/security.c | 63 +++++++++++++++++++++
> 6 files changed, 297 insertions(+)
>
> --- linux-2.6.27-rc6.orig/fs/namei.c
> +++ linux-2.6.27-rc6/fs/namei.c
> @@ -1585,6 +1585,10 @@ int may_open(struct nameidata *nd, int a
> * Refuse to truncate files with mandatory locks held on them.
> */
> error = locks_verify_locked(inode);
> + if (!error)
> + error = security_path_truncate(&nd->path, 0,
> + ATTR_MTIME|ATTR_CTIME|ATTR_OPEN,
> + NULL);
> if (!error) {
> DQUOT_INIT(inode);
>
> @@ -1615,7 +1619,11 @@ static int __open_namei_create(struct na
>
> if (!IS_POSIXACL(dir->d_inode))
> mode &= ~current->fs->umask;
> + error = security_path_mknod(&nd->path, path->dentry, mode, 0);
> + if (error)
> + goto out_unlock;
> error = vfs_create(dir->d_inode, path->dentry, mode, nd);
> +out_unlock:
> mutex_unlock(&dir->d_inode->i_mutex);
> dput(nd->path.dentry);
> nd->path.dentry = path->dentry;
> @@ -2018,6 +2026,9 @@ asmlinkage long sys_mknodat(int dfd, con
> error = mnt_want_write(nd.path.mnt);
> if (error)
> goto out_dput;
> + error = security_path_mknod(&nd.path, dentry, mode, dev);
> + if (error)
> + goto out_drop_write;
> switch (mode & S_IFMT) {
> case 0: case S_IFREG:
> error = vfs_create(nd.path.dentry->d_inode,dentry,mode,&nd);
> @@ -2030,6 +2041,7 @@ asmlinkage long sys_mknodat(int dfd, con
> error = vfs_mknod(nd.path.dentry->d_inode,dentry,mode,0);
> break;
> }
> +out_drop_write:
> mnt_drop_write(nd.path.mnt);
> out_dput:
> dput(dentry);
> @@ -2089,7 +2101,11 @@ asmlinkage long sys_mkdirat(int dfd, con
> error = mnt_want_write(nd.path.mnt);
> if (error)
> goto out_dput;
> + error = security_path_mkdir(&nd.path, dentry, mode);
> + if (error)
> + goto out_drop_write;
> error = vfs_mkdir(nd.path.dentry->d_inode, dentry, mode);
> +out_drop_write:
> mnt_drop_write(nd.path.mnt);
> out_dput:
> dput(dentry);
> @@ -2196,7 +2212,11 @@ static long do_rmdir(int dfd, const char
> error = mnt_want_write(nd.path.mnt);
> if (error)
> goto exit3;
> + error = security_path_rmdir(&nd.path, dentry);
> + if (error)
> + goto exit4;
> error = vfs_rmdir(nd.path.dentry->d_inode, dentry);
> +exit4:
> mnt_drop_write(nd.path.mnt);
> exit3:
> dput(dentry);
> @@ -2278,7 +2298,11 @@ static long do_unlinkat(int dfd, const c
> error = mnt_want_write(nd.path.mnt);
> if (error)
> goto exit2;
> + error = security_path_unlink(&nd.path, dentry);
> + if (error)
> + goto exit3;
> error = vfs_unlink(nd.path.dentry->d_inode, dentry);
> +exit3:
> mnt_drop_write(nd.path.mnt);
> exit2:
> dput(dentry);
> @@ -2359,7 +2383,11 @@ asmlinkage long sys_symlinkat(const char
> error = mnt_want_write(nd.path.mnt);
> if (error)
> goto out_dput;
> + error = security_path_symlink(&nd.path, dentry, from);
> + if (error)
> + goto out_drop_write;
> error = vfs_symlink(nd.path.dentry->d_inode, dentry, from);
> +out_drop_write:
> mnt_drop_write(nd.path.mnt);
> out_dput:
> dput(dentry);
> @@ -2456,7 +2484,11 @@ asmlinkage long sys_linkat(int olddfd, c
> error = mnt_want_write(nd.path.mnt);
> if (error)
> goto out_dput;
> + error = security_path_link(old_path.dentry, &nd.path, new_dentry);
> + if (error)
> + goto out_drop_write;
> error = vfs_link(old_path.dentry, nd.path.dentry->d_inode, new_dentry);
> +out_drop_write:
> mnt_drop_write(nd.path.mnt);
> out_dput:
> dput(new_dentry);
> @@ -2688,8 +2720,13 @@ asmlinkage long sys_renameat(int olddfd,
> error = mnt_want_write(oldnd.path.mnt);
> if (error)
> goto exit5;
> + error = security_path_rename(&oldnd.path, old_dentry,
> + &newnd.path, new_dentry);
> + if (error)
> + goto exit6;
> error = vfs_rename(old_dir->d_inode, old_dentry,
> new_dir->d_inode, new_dentry);
> +exit6:
> mnt_drop_write(oldnd.path.mnt);
> exit5:
> dput(new_dentry);
> --- linux-2.6.27-rc6.orig/fs/open.c
> +++ linux-2.6.27-rc6/fs/open.c
> @@ -272,6 +272,8 @@ static long do_sys_truncate(const char _
> goto put_write_and_out;
>
> error = locks_verify_truncate(inode, NULL, length);
> + if (!error)
> + error = security_path_truncate(&path, length, 0, NULL);
> if (!error) {
> DQUOT_INIT(inode);
> error = do_truncate(path.dentry, length, 0, NULL);
> @@ -329,6 +331,9 @@ static long do_sys_ftruncate(unsigned in
>
> error = locks_verify_truncate(inode, file, length);
> if (!error)
> + error = security_path_truncate(&file->f_path, length,
> + ATTR_MTIME|ATTR_CTIME, file);
> + if (!error)
> error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, file);
> out_putf:
> fput(file);
> --- linux-2.6.27-rc6.orig/include/linux/security.h
> +++ linux-2.6.27-rc6/include/linux/security.h
> @@ -345,17 +345,37 @@ static inline void security_free_mnt_opt
> * @dir contains the inode structure of the parent directory of the new link.
> * @new_dentry contains the dentry structure for the new link.
> * Return 0 if permission is granted.
> + * @path_link:
> + * Check permission before creating a new hard link to a file.
> + * @old_dentry contains the dentry structure for an existing link
> + * to the file.
> + * @new_dir contains the path structure of the parent directory of
> + * the new link.
> + * @new_dentry contains the dentry structure for the new link.
> + * Return 0 if permission is granted.
> * @inode_unlink:
> * Check the permission to remove a hard link to a file.
> * @dir contains the inode structure of parent directory of the file.
> * @dentry contains the dentry structure for file to be unlinked.
> * Return 0 if permission is granted.
> + * @path_unlink:
> + * Check the permission to remove a hard link to a file.
> + * @dir contains the path structure of parent directory of the file.
> + * @dentry contains the dentry structure for file to be unlinked.
> + * Return 0 if permission is granted.
> * @inode_symlink:
> * Check the permission to create a symbolic link to a file.
> * @dir contains the inode structure of parent directory of the symbolic link.
> * @dentry contains the dentry structure of the symbolic link.
> * @old_name contains the pathname of file.
> * Return 0 if permission is granted.
> + * @path_symlink:
> + * Check the permission to create a symbolic link to a file.
> + * @dir contains the path structure of parent directory of
> + * the symbolic link.
> + * @dentry contains the dentry structure of the symbolic link.
> + * @old_name contains the pathname of file.
> + * Return 0 if permission is granted.
> * @inode_mkdir:
> * Check permissions to create a new directory in the existing directory
> * associated with inode strcture @dir.
> @@ -363,11 +383,25 @@ static inline void security_free_mnt_opt
> * @dentry contains the dentry structure of new directory.
> * @mode contains the mode of new directory.
> * Return 0 if permission is granted.
> + * @path_mkdir:
> + * Check permissions to create a new directory in the existing directory
> + * associated with path strcture @path.
> + * @dir containst the path structure of parent of the directory
> + * to be created.
> + * @dentry contains the dentry structure of new directory.
> + * @mode contains the mode of new directory.
> + * Return 0 if permission is granted.
> * @inode_rmdir:
> * Check the permission to remove a directory.
> * @dir contains the inode structure of parent of the directory to be removed.
> * @dentry contains the dentry structure of directory to be removed.
> * Return 0 if permission is granted.
> + * @path_rmdir:
> + * Check the permission to remove a directory.
> + * @dir contains the path structure of parent of the directory to be
> + * removed.
> + * @dentry contains the dentry structure of directory to be removed.
> + * Return 0 if permission is granted.
> * @inode_mknod:
> * Check permissions when creating a special file (or a socket or a fifo
> * file created via the mknod system call). Note that if mknod operation
> @@ -378,6 +412,15 @@ static inline void security_free_mnt_opt
> * @mode contains the mode of the new file.
> * @dev contains the device number.
> * Return 0 if permission is granted.
> + * @path_mknod:
> + * Check permissions when creating a file. Note that this hook is called
> + * even if mknod operation is being done for a regular file.
> + * @dir contains the path structure of parent of the new file.
> + * @dentry contains the dentry structure of the new file.
> + * @mode contains the mode of the new file.
> + * @dev contains the undecoded device number. Use new_decode_dev() to get
> + * the decoded device number.
> + * Return 0 if permission is granted.
> * @inode_rename:
> * Check for permission to rename a file or directory.
> * @old_dir contains the inode structure for parent of the old link.
> @@ -385,6 +428,13 @@ static inline void security_free_mnt_opt
> * @new_dir contains the inode structure for parent of the new link.
> * @new_dentry contains the dentry structure of the new link.
> * Return 0 if permission is granted.
> + * @path_rename:
> + * Check for permission to rename a file or directory.
> + * @old_dir contains the path structure for parent of the old link.
> + * @old_dentry contains the dentry structure of the old link.
> + * @new_dir contains the path structure for parent of the new link.
> + * @new_dentry contains the dentry structure of the new link.
> + * Return 0 if permission is granted.
> * @inode_readlink:
> * Check the permission to read the symbolic link.
> * @dentry contains the dentry structure for the file link.
> @@ -413,6 +463,13 @@ static inline void security_free_mnt_opt
> * @dentry contains the dentry structure for the file.
> * @attr is the iattr structure containing the new file attributes.
> * Return 0 if permission is granted.
> + * @path_truncate:
> + * Check permission before truncating a file.
> + * @path contains the path structure for the file.
> + * @length is the new length of the file.
> + * @time_attrs is the flags passed to do_truncate().
> + * @filp is the file structure (may be NULL).
> + * Return 0 if permission is granted.
> * @inode_getattr:
> * Check permission before obtaining file attributes.
> * @mnt is the vfsmount where the dentry was looked up
> @@ -1350,6 +1407,20 @@ struct security_operations {
> struct super_block *newsb);
> int (*sb_parse_opts_str) (char *options, struct security_mnt_opts *opts);
>
> + int (*path_unlink) (struct path *dir, struct dentry *dentry);
> + int (*path_mkdir) (struct path *dir, struct dentry *dentry, int mode);
> + int (*path_rmdir) (struct path *dir, struct dentry *dentry);
> + int (*path_mknod) (struct path *dir, struct dentry *dentry, int mode,
> + unsigned int dev);
> + int (*path_truncate) (struct path *path, loff_t length,
> + unsigned int time_attrs, struct file *filp);
> + int (*path_symlink) (struct path *dir, struct dentry *dentry,
> + const char *old_name);
> + int (*path_link) (struct dentry *old_dentry, struct path *new_dir,
> + struct dentry *new_dentry);
> + int (*path_rename) (struct path *old_dir, struct dentry *old_dentry,
> + struct path *new_dir, struct dentry *new_dentry);
> +
> int (*inode_alloc_security) (struct inode *inode);
> void (*inode_free_security) (struct inode *inode);
> int (*inode_init_security) (struct inode *inode, struct inode *dir,
> @@ -1618,6 +1689,20 @@ void security_sb_clone_mnt_opts(const st
> struct super_block *newsb);
> int security_sb_parse_opts_str(char *options, struct security_mnt_opts *opts);
>
> +int security_path_unlink(struct path *dir, struct dentry *dentry);
> +int security_path_mkdir(struct path *dir, struct dentry *dentry, int mode);
> +int security_path_rmdir(struct path *dir, struct dentry *dentry);
> +int security_path_mknod(struct path *dir, struct dentry *dentry, int mode,
> + unsigned int dev);
> +int security_path_truncate(struct path *path, loff_t length,
> + unsigned int time_attrs, struct file *filp);
> +int security_path_symlink(struct path *dir, struct dentry *dentry,
> + const char *old_name);
> +int security_path_link(struct dentry *old_dentry, struct path *new_dir,
> + struct dentry *new_dentry);
> +int security_path_rename(struct path *old_dir, struct dentry *old_dentry,
> + struct path *new_dir, struct dentry *new_dentry);
> +
> int security_inode_alloc(struct inode *inode);
> void security_inode_free(struct inode *inode);
> int security_inode_init_security(struct inode *inode, struct inode *dir,
> @@ -1949,6 +2034,56 @@ static inline int security_sb_parse_opts
> return 0;
> }
>
> +static inline int security_path_unlink(struct path *dir, struct dentry *dentry)
> +{
> + return 0;
> +}
> +
> +static inline int security_path_mkdir(struct path *dir, struct dentry *dentry,
> + int mode)
> +{
> + return 0;
> +}
> +
> +static inline int security_path_rmdir(struct path *dir, struct dentry *dentry)
> +{
> + return 0;
> +}
> +
> +static inline int security_path_mknod(struct path *dir, struct dentry *dentry,
> + int mode, unsigned int dev)
> +{
> + return 0;
> +}
> +
> +static inline int security_path_truncate(struct path *path, loff_t length,
> + unsigned int time_attrs,
> + struct file *filp)
> +{
> + return 0;
> +}
> +
> +static inline int security_path_symlink(struct path *dir, struct dentry *dentry,
> + const char *old_name)
> +{
> + return 0;
> +}
> +
> +static inline int security_path_link(struct dentry *old_dentry,
> + struct path *new_dir,
> + struct dentry *new_dentry)
> +{
> + return 0;
> +}
> +
> +static inline int security_path_rename(struct path *old_dir,
> + struct dentry *old_dentry,
> + struct path *new_dir,
> + struct dentry *new_dentry)
> +{
> + return 0;
> +}
> +
> static inline int security_inode_alloc(struct inode *inode)
> {
> return 0;
> --- linux-2.6.27-rc6.orig/net/unix/af_unix.c
> +++ linux-2.6.27-rc6/net/unix/af_unix.c
> @@ -827,7 +827,11 @@ static int unix_bind(struct socket *sock
> err = mnt_want_write(nd.path.mnt);
> if (err)
> goto out_mknod_dput;
> + err = security_path_mknod(&nd.path, dentry, mode, 0);
> + if (err)
> + goto out_mknod_drop_write;
> err = vfs_mknod(nd.path.dentry->d_inode, dentry, mode, 0);
> +out_mknod_drop_write:
> mnt_drop_write(nd.path.mnt);
> if (err)
> goto out_mknod_dput;
> --- linux-2.6.27-rc6.orig/security/capability.c
> +++ linux-2.6.27-rc6/security/capability.c
> @@ -268,6 +268,51 @@ static void cap_inode_getsecid(const str
> *secid = 0;
> }
>
> +static int cap_path_mknod(struct path *dir, struct dentry *dentry, int mode,
> + unsigned int dev)
> +{
> + return 0;
> +}
> +
> +static int cap_path_mkdir(struct path *dir, struct dentry *dentry, int mode)
> +{
> + return 0;
> +}
> +
> +static int cap_path_rmdir(struct path *dir, struct dentry *dentry)
> +{
> + return 0;
> +}
> +
> +static int cap_path_unlink(struct path *dir, struct dentry *dentry)
> +{
> + return 0;
> +}
> +
> +static int cap_path_symlink(struct path *dir, struct dentry *dentry,
> + const char *old_name)
> +{
> + return 0;
> +}
> +
> +static int cap_path_link(struct dentry *old_dentry, struct path *new_dir,
> + struct dentry *new_dentry)
> +{
> + return 0;
> +}
> +
> +static int cap_path_rename(struct path *old_path, struct dentry *old_dentry,
> + struct path *new_path, struct dentry *new_dentry)
> +{
> + return 0;
> +}
> +
> +static int cap_path_truncate(struct path *path, loff_t length,
> + unsigned int time_attrs, struct file *filp)
> +{
> + return 0;
> +}
> +
> static int cap_file_permission(struct file *file, int mask)
> {
> return 0;
> @@ -877,6 +922,14 @@ void security_fixup_ops(struct security_
> set_to_cap_if_null(ops, inode_setsecurity);
> set_to_cap_if_null(ops, inode_listsecurity);
> set_to_cap_if_null(ops, inode_getsecid);
> + set_to_cap_if_null(ops, path_mknod);
> + set_to_cap_if_null(ops, path_mkdir);
> + set_to_cap_if_null(ops, path_rmdir);
> + set_to_cap_if_null(ops, path_unlink);
> + set_to_cap_if_null(ops, path_symlink);
> + set_to_cap_if_null(ops, path_link);
> + set_to_cap_if_null(ops, path_rename);
> + set_to_cap_if_null(ops, path_truncate);
> set_to_cap_if_null(ops, file_permission);
> set_to_cap_if_null(ops, file_alloc_security);
> set_to_cap_if_null(ops, file_free_security);
> --- linux-2.6.27-rc6.orig/security/security.c
> +++ linux-2.6.27-rc6/security/security.c
> @@ -358,6 +358,69 @@ int security_inode_init_security(struct
> }
> EXPORT_SYMBOL(security_inode_init_security);
>
> +int security_path_mknod(struct path *path, struct dentry *dentry, int mode,
> + unsigned int dev)
> +{
> + if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
> + return 0;
> + return security_ops->path_mknod(path, dentry, mode, dev);
> +}
> +
> +int security_path_mkdir(struct path *path, struct dentry *dentry, int mode)
> +{
> + if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
> + return 0;
> + return security_ops->path_mkdir(path, dentry, mode);
> +}
> +
> +int security_path_rmdir(struct path *path, struct dentry *dentry)
> +{
> + if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
> + return 0;
> + return security_ops->path_rmdir(path, dentry);
> +}
> +
> +int security_path_unlink(struct path *path, struct dentry *dentry)
> +{
> + if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
> + return 0;
> + return security_ops->path_unlink(path, dentry);
> +}
> +
> +int security_path_symlink(struct path *path, struct dentry *dentry,
> + const char *old_name)
> +{
> + if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
> + return 0;
> + return security_ops->path_symlink(path, dentry, old_name);
> +}
> +
> +int security_path_link(struct dentry *old_dentry, struct path *new_dir,
> + struct dentry *new_dentry)
> +{
> + if (unlikely(IS_PRIVATE(old_dentry->d_inode)))
> + return 0;
> + return security_ops->path_link(old_dentry, new_dir, new_dentry);
> +}
> +
> +int security_path_rename(struct path *old_dir, struct dentry *old_dentry,
> + struct path *new_dir, struct dentry *new_dentry)
> +{
> + if (unlikely(IS_PRIVATE(old_dentry->d_inode) ||
> + (new_dentry->d_inode && IS_PRIVATE(new_dentry->d_inode))))
> + return 0;
> + return security_ops->path_rename(old_dir, old_dentry, new_dir,
> + new_dentry);
> +}
> +
> +int security_path_truncate(struct path *path, loff_t length,
> + unsigned int time_attrs, struct file *filp)
> +{
> + if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
> + return 0;
> + return security_ops->path_truncate(path, length, time_attrs, filp);
> +}
> +
> int security_inode_create(struct inode *dir, struct dentry *dentry, int mode)
> {
> if (unlikely(IS_PRIVATE(dir)))
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Introduce new LSM hooks where vfsmount is available.
2008-09-17 4:00 ` Casey Schaufler
@ 2008-09-17 8:35 ` Kentaro Takeda
2008-09-17 17:46 ` david
1 sibling, 0 replies; 16+ messages in thread
From: Kentaro Takeda @ 2008-09-17 8:35 UTC (permalink / raw)
To: Casey Schaufler
Cc: linux-security-module, linux-fsdevel, linux-kernel,
Toshiharu Harada
Casey Schaufler wrote:
> As always, the community will be eager to see the code that makes
> use of these hooks.
We are ready for submitting body of TOMOYO Linux as always. :-)
However, the largest stumbling block of merging TOMOYO Linux is the
method for calculating pathname in LSM module. This patch contains
all modifications to the existing kernel code by TOMOYO Linux (the
rest modification is within security/tomoyo).
We'd like to ask the community's approval of this approach before
review of TOMOYO Linux main module.
Regards,
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Introduce new LSM hooks where vfsmount is available.
2008-09-17 4:00 ` Casey Schaufler
2008-09-17 8:35 ` Kentaro Takeda
@ 2008-09-17 17:46 ` david
1 sibling, 0 replies; 16+ messages in thread
From: david @ 2008-09-17 17:46 UTC (permalink / raw)
To: Casey Schaufler
Cc: Kentaro Takeda, linux-security-module, linux-fsdevel,
linux-kernel, Toshiharu Harada
On Tue, 16 Sep 2008, Casey Schaufler wrote:
> Kentaro Takeda wrote:
>> TOMOYO Linux needs method for calculating pathname in LSM module.
>> However, we have received comment from Al Viro, the vfs maintainer,
>> that adding vfsmount parameter to vfs helper functions (and LSM hooks)
>> is not preferable. We have asked some people (including Al), and we
>> came back to the most straightforward approach; adding new LSM hooks
>> where vfsmount is available.
>>
>> The attached patch introduces several new LSM hooks TOMOYO Linux
>> needs. It has less impact to existing LSM module and no impact to vfs
>> helper functions. Please review it.
>>
>> Regards,
>>
>>
> As always, the community will be eager to see the code that makes
> use of these hooks.
you can bet that if these hooks are accepted, AppArmor and the various
other LSMs that have not been accepted becouse of the problems they have
dealing with paths will switch to using the 'approved' hooks.
so there are several bodies of code that have already been published that
would want to use hooks like these if they are available.
David Lang
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Introduce new LSM hooks where vfsmount is available.
2008-09-17 2:16 ` [PATCH] Introduce " Kentaro Takeda
2008-09-17 4:00 ` Casey Schaufler
@ 2008-09-23 6:06 ` James Morris
2008-09-23 6:12 ` david
2008-09-24 8:00 ` Kentaro Takeda
1 sibling, 2 replies; 16+ messages in thread
From: James Morris @ 2008-09-23 6:06 UTC (permalink / raw)
To: Kentaro Takeda
Cc: linux-security-module, linux-fsdevel, linux-kernel,
Toshiharu Harada
On Wed, 17 Sep 2008, Kentaro Takeda wrote:
> TOMOYO Linux needs method for calculating pathname in LSM module.
> However, we have received comment from Al Viro, the vfs maintainer,
> that adding vfsmount parameter to vfs helper functions (and LSM hooks)
> is not preferable. We have asked some people (including Al), and we
> came back to the most straightforward approach; adding new LSM hooks
> where vfsmount is available.
>
> The attached patch introduces several new LSM hooks TOMOYO Linux
> needs. It has less impact to existing LSM module and no impact to vfs
> helper functions. Please review it.
I don't see any technical errors in this patch.
If it is going to be merged, please make a new config option for
path-based hooks (similar to that for the network hooks), so they can be
compiled out.
- James
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Introduce new LSM hooks where vfsmount is available.
2008-09-23 6:06 ` James Morris
@ 2008-09-23 6:12 ` david
2008-09-23 15:02 ` Stephen Smalley
2008-09-24 8:00 ` Kentaro Takeda
1 sibling, 1 reply; 16+ messages in thread
From: david @ 2008-09-23 6:12 UTC (permalink / raw)
To: James Morris
Cc: Kentaro Takeda, linux-security-module, linux-fsdevel,
linux-kernel, Toshiharu Harada
On Tue, 23 Sep 2008, James Morris wrote:
> On Wed, 17 Sep 2008, Kentaro Takeda wrote:
>
>> TOMOYO Linux needs method for calculating pathname in LSM module.
>> However, we have received comment from Al Viro, the vfs maintainer,
>> that adding vfsmount parameter to vfs helper functions (and LSM hooks)
>> is not preferable. We have asked some people (including Al), and we
>> came back to the most straightforward approach; adding new LSM hooks
>> where vfsmount is available.
>>
>> The attached patch introduces several new LSM hooks TOMOYO Linux
>> needs. It has less impact to existing LSM module and no impact to vfs
>> helper functions. Please review it.
>
> I don't see any technical errors in this patch.
>
> If it is going to be merged, please make a new config option for
> path-based hooks (similar to that for the network hooks), so they can be
> compiled out.
one question about these new hook locations.
it is possible to gather all the info that was gathered at the old hook
locations from the new ones? I realize that you are not eliminating the
old hooks (and possibly can't for backwards compatibility), but possibly
they should be depriciated in favor of the new locations if they can
satisfy both the old uses and new use cases.
David Lang
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Introduce new LSM hooks where vfsmount is available.
2008-09-23 6:12 ` david
@ 2008-09-23 15:02 ` Stephen Smalley
2008-09-23 15:20 ` Alexey Dobriyan
0 siblings, 1 reply; 16+ messages in thread
From: Stephen Smalley @ 2008-09-23 15:02 UTC (permalink / raw)
To: david
Cc: James Morris, Kentaro Takeda, linux-security-module,
linux-fsdevel, linux-kernel, Toshiharu Harada
On Mon, 2008-09-22 at 23:12 -0700, david@lang.hm wrote:
> On Tue, 23 Sep 2008, James Morris wrote:
>
> > On Wed, 17 Sep 2008, Kentaro Takeda wrote:
> >
> >> TOMOYO Linux needs method for calculating pathname in LSM module.
> >> However, we have received comment from Al Viro, the vfs maintainer,
> >> that adding vfsmount parameter to vfs helper functions (and LSM hooks)
> >> is not preferable. We have asked some people (including Al), and we
> >> came back to the most straightforward approach; adding new LSM hooks
> >> where vfsmount is available.
> >>
> >> The attached patch introduces several new LSM hooks TOMOYO Linux
> >> needs. It has less impact to existing LSM module and no impact to vfs
> >> helper functions. Please review it.
> >
> > I don't see any technical errors in this patch.
> >
> > If it is going to be merged, please make a new config option for
> > path-based hooks (similar to that for the network hooks), so they can be
> > compiled out.
>
> one question about these new hook locations.
>
> it is possible to gather all the info that was gathered at the old hook
> locations from the new ones? I realize that you are not eliminating the
> old hooks (and possibly can't for backwards compatibility), but possibly
> they should be depriciated in favor of the new locations if they can
> satisfy both the old uses and new use cases.
They can't.
--
Stephen Smalley
National Security Agency
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Introduce new LSM hooks where vfsmount is available.
2008-09-23 15:02 ` Stephen Smalley
@ 2008-09-23 15:20 ` Alexey Dobriyan
2008-09-23 21:20 ` James Morris
0 siblings, 1 reply; 16+ messages in thread
From: Alexey Dobriyan @ 2008-09-23 15:20 UTC (permalink / raw)
To: Stephen Smalley
Cc: david, James Morris, Kentaro Takeda, linux-security-module,
linux-fsdevel, linux-kernel, Toshiharu Harada
On Tue, Sep 23, 2008 at 11:02:47AM -0400, Stephen Smalley wrote:
>
> On Mon, 2008-09-22 at 23:12 -0700, david@lang.hm wrote:
> > On Tue, 23 Sep 2008, James Morris wrote:
> >
> > > On Wed, 17 Sep 2008, Kentaro Takeda wrote:
> > >
> > >> TOMOYO Linux needs method for calculating pathname in LSM module.
> > >> However, we have received comment from Al Viro, the vfs maintainer,
> > >> that adding vfsmount parameter to vfs helper functions (and LSM hooks)
> > >> is not preferable. We have asked some people (including Al), and we
> > >> came back to the most straightforward approach; adding new LSM hooks
> > >> where vfsmount is available.
> > >>
> > >> The attached patch introduces several new LSM hooks TOMOYO Linux
> > >> needs. It has less impact to existing LSM module and no impact to vfs
> > >> helper functions. Please review it.
> > >
> > > I don't see any technical errors in this patch.
> > >
> > > If it is going to be merged, please make a new config option for
> > > path-based hooks (similar to that for the network hooks), so they can be
> > > compiled out.
Another pointless config option.
It's actually pretty surprising that SECURITY_NETWORK is
a) user-visible and b) is not SECURITY && NET.
Same for SECURITY_NETWORK_XFRM.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Introduce new LSM hooks where vfsmount is available.
2008-09-23 15:20 ` Alexey Dobriyan
@ 2008-09-23 21:20 ` James Morris
0 siblings, 0 replies; 16+ messages in thread
From: James Morris @ 2008-09-23 21:20 UTC (permalink / raw)
To: Alexey Dobriyan
Cc: Stephen Smalley, david, Kentaro Takeda, linux-security-module,
linux-fsdevel, linux-kernel, Toshiharu Harada
On Tue, 23 Sep 2008, Alexey Dobriyan wrote:
> > > > I don't see any technical errors in this patch.
> > > >
> > > > If it is going to be merged, please make a new config option for
> > > > path-based hooks (similar to that for the network hooks), so they can be
> > > > compiled out.
>
> Another pointless config option.
Why is it pointless? If distros don't want to use these hooks, they
shouldn't have to have them enabled.
>
> It's actually pretty surprising that SECURITY_NETWORK is
> a) user-visible and b) is not SECURITY && NET.
>
> Same for SECURITY_NETWORK_XFRM.
IIRC, this option was a perquisite for merge. Networking hooks have a
relatively high overhead and not everyone wants to use them, even if they
have networking enabled.
- James
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Introduce new LSM hooks where vfsmount is available.
2008-09-23 6:06 ` James Morris
2008-09-23 6:12 ` david
@ 2008-09-24 8:00 ` Kentaro Takeda
2008-09-24 13:28 ` Serge E. Hallyn
1 sibling, 1 reply; 16+ messages in thread
From: Kentaro Takeda @ 2008-09-24 8:00 UTC (permalink / raw)
To: James Morris, Andrew Morton
Cc: linux-security-module, linux-fsdevel, linux-kernel,
Toshiharu Harada
James Morris wrote:
> I don't see any technical errors in this patch.
Thanks for your review.
> If it is going to be merged, please make a new config option for
> path-based hooks (similar to that for the network hooks), so they can be
> compiled out.
I see, here it is.
---
Subject: introduce new LSM hooks where vfsmount is available.
----- What is this patch for? -----
There are security_inode_*() LSM hooks for attribute-based MAC, but they are not
suitable for pathname-based MAC because they don't receive "struct vfsmount"
information.
----- How this patch was developed? -----
Two pathname-based MACs, AppArmor and TOMOYO Linux, are trying to merge
upstream. But because of "struct vfsmount" problem, they have been unable to
merge upstream.
Here are the list of approaches and the reasons of denial.
(1) Not using LSM
http://lwn.net/Articles/277833/
This approach was rejected because security modules should use LSM because the
whole idea behind LSM was to have a single set of hooks for all security
modules; if every module now adds its own set of hooks, that purpose will have
been defeated and the kernel will turn into a big mess of security hooks.
(2) Retrieving "struct vfsmount" from "struct task_struct".
http://lkml.org/lkml/2007/11/5/388
Since "struct task_struct" contains list of "struct vfsmount",
"struct vfsmount" which corresponds to "struct dentry" can be retrieved from
the list unless "mount --bind" is used.
This approach turned out to cause a critical problem that getting namespace_sem
lock from security_inode_*() triggers AB-BA deadlock.
(3) Adding "struct vfsmount" parameter to VFS helper functions.
http://lkml.org/lkml/2008/5/29/207
This approach adds "struct vfsmount" to VFS helper functions (e.g. vfs_mkdir()
and vfs_symlink()) and LSM hooks inside VFS helper functions. This approach is
helpful for not only AppArmor and TOMOYO Linux 2.x but also SELinux and
auditing purpose, for this approach allows existent LSM users to use pathnames
in their access control and audit logs.
This approach was rejected by Al Viro, the VFS maintainer, because he thinks
individual filesystem should remain "struct vfsmount"-unaware and VFS helper
functions should not receive "struct vfsmount".
Al Viro also suggested to move existing security_inode_*() to out of VFS
helper functions so that security_inode_*() can receive "struct vfsmount"
without modifying VFS helper functions, but this suggestion was opposed by
Stephen Smalley because changing the order of permission checks (i.e.
MAC checks before DAC checks) is not acceptable.
(4) Passing "struct vfsmount" via "struct task_struct".
http://lkml.org/lkml/2007/11/16/157
Since we didn't understand the reason why accessing "struct vfsmount" from
LSM hooks inside VFS helper functions is not acceptable, we thought the reason
why VFS helper functions don't receive "struct vfsmount" is the amount of
modifications needed to do so. Thus, we proposed to pass "struct vfsmount" via
"struct task_struct" so that modifications remain minimal.
This approach was rejected because this is an abuse of "struct task_struct".
(5) Remembering pathname of "struct vfsmount" via "struct task_struct".
http://lkml.org/lkml/2008/8/19/16
Since pathname of a "struct dentry" up to the mount point can be calculated
without "struct vfsmount", absolute pathname of a "struct dentry" can be
calculated if "struct task_struct" can remember absolute pathname of a
"struct vfsmount" which corresponds to "struct dentry".
As we now understand that Al Viro is opposing to access "struct vfsmount" from
LSM hooks inside VFS helper functions, we gave up delivering "struct vfsmount"
to LSM hooks inside VFS helper functions.
Kernel 2.6.26 introduced read-only bind mount feature, and hooks for that
feature (i.e. mnt_want_write() and mnt_drop_write()) were inserted around
VFS helper functions call. Since mnt_want_write() receives "struct vfsmount"
which corresponds to "struct dentry" that will be passed to subsequent VFS
helper functions call, we associated pathname of "struct vfsmount" with
"struct task_struct" instead of associating "struct vfsmount" itself.
This approach was not explicitly rejected, but there seems to be performance
problem.
(6) Introducing new LSM hooks.
(this patch)
We understand that adding new LSM hooks which receive "struct vfsmount" outside
VFS helper functions is the most straightforward approach. This approach has
less impact to existing LSM module and no impact to VFS helper functions.
Signed-off-by: Kentaro Takeda <takedakn@nttdata.co.jp>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Toshiharu Harada <haradats@nttdata.co.jp>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Crispin Cowan <crispin@crispincowan.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Casey Schaufler <casey@schaufler-ca.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
-----
fs/namei.c | 37 ++++++++++++
fs/open.c | 5 +
include/linux/security.h | 139 +++++++++++++++++++++++++++++++++++++++++++++++
net/unix/af_unix.c | 4 +
security/Kconfig | 9 +++
security/capability.c | 57 +++++++++++++++++++
security/security.c | 66 ++++++++++++++++++++++
7 files changed, 317 insertions(+)
--- linux-2.6.27-rc7-mm1.orig/fs/namei.c
+++ linux-2.6.27-rc7-mm1/fs/namei.c
@@ -1571,6 +1571,10 @@ int may_open(struct nameidata *nd, int a
* Refuse to truncate files with mandatory locks held on them.
*/
error = locks_verify_locked(inode);
+ if (!error)
+ error = security_path_truncate(&nd->path, 0,
+ ATTR_MTIME|ATTR_CTIME|ATTR_OPEN,
+ NULL);
if (!error) {
DQUOT_INIT(inode);
@@ -1601,7 +1605,11 @@ static int __open_namei_create(struct na
if (!IS_POSIXACL(dir->d_inode))
mode &= ~current->fs->umask;
+ error = security_path_mknod(&nd->path, path->dentry, mode, 0);
+ if (error)
+ goto out_unlock;
error = vfs_create(dir->d_inode, path->dentry, mode, nd);
+out_unlock:
mutex_unlock(&dir->d_inode->i_mutex);
dput(nd->path.dentry);
nd->path.dentry = path->dentry;
@@ -2014,6 +2022,9 @@ asmlinkage long sys_mknodat(int dfd, con
error = mnt_want_write(nd.path.mnt);
if (error)
goto out_dput;
+ error = security_path_mknod(&nd.path, dentry, mode, dev);
+ if (error)
+ goto out_drop_write;
switch (mode & S_IFMT) {
case 0: case S_IFREG:
error = vfs_create(nd.path.dentry->d_inode,dentry,mode,&nd);
@@ -2026,6 +2037,7 @@ asmlinkage long sys_mknodat(int dfd, con
error = vfs_mknod(nd.path.dentry->d_inode,dentry,mode,0);
break;
}
+out_drop_write:
mnt_drop_write(nd.path.mnt);
out_dput:
dput(dentry);
@@ -2085,7 +2097,11 @@ asmlinkage long sys_mkdirat(int dfd, con
error = mnt_want_write(nd.path.mnt);
if (error)
goto out_dput;
+ error = security_path_mkdir(&nd.path, dentry, mode);
+ if (error)
+ goto out_drop_write;
error = vfs_mkdir(nd.path.dentry->d_inode, dentry, mode);
+out_drop_write:
mnt_drop_write(nd.path.mnt);
out_dput:
dput(dentry);
@@ -2192,7 +2208,11 @@ static long do_rmdir(int dfd, const char
error = mnt_want_write(nd.path.mnt);
if (error)
goto exit3;
+ error = security_path_rmdir(&nd.path, dentry);
+ if (error)
+ goto exit4;
error = vfs_rmdir(nd.path.dentry->d_inode, dentry);
+exit4:
mnt_drop_write(nd.path.mnt);
exit3:
dput(dentry);
@@ -2274,7 +2294,11 @@ static long do_unlinkat(int dfd, const c
error = mnt_want_write(nd.path.mnt);
if (error)
goto exit2;
+ error = security_path_unlink(&nd.path, dentry);
+ if (error)
+ goto exit3;
error = vfs_unlink(nd.path.dentry->d_inode, dentry);
+exit3:
mnt_drop_write(nd.path.mnt);
exit2:
dput(dentry);
@@ -2355,7 +2379,11 @@ asmlinkage long sys_symlinkat(const char
error = mnt_want_write(nd.path.mnt);
if (error)
goto out_dput;
+ error = security_path_symlink(&nd.path, dentry, from);
+ if (error)
+ goto out_drop_write;
error = vfs_symlink(nd.path.dentry->d_inode, dentry, from);
+out_drop_write:
mnt_drop_write(nd.path.mnt);
out_dput:
dput(dentry);
@@ -2452,7 +2480,11 @@ asmlinkage long sys_linkat(int olddfd, c
error = mnt_want_write(nd.path.mnt);
if (error)
goto out_dput;
+ error = security_path_link(old_path.dentry, &nd.path, new_dentry);
+ if (error)
+ goto out_drop_write;
error = vfs_link(old_path.dentry, nd.path.dentry->d_inode, new_dentry);
+out_drop_write:
mnt_drop_write(nd.path.mnt);
out_dput:
dput(new_dentry);
@@ -2682,8 +2714,13 @@ asmlinkage long sys_renameat(int olddfd,
error = mnt_want_write(oldnd.path.mnt);
if (error)
goto exit5;
+ error = security_path_rename(&oldnd.path, old_dentry,
+ &newnd.path, new_dentry);
+ if (error)
+ goto exit6;
error = vfs_rename(old_dir->d_inode, old_dentry,
new_dir->d_inode, new_dentry);
+exit6:
mnt_drop_write(oldnd.path.mnt);
exit5:
dput(new_dentry);
--- linux-2.6.27-rc7-mm1.orig/fs/open.c
+++ linux-2.6.27-rc7-mm1/fs/open.c
@@ -272,6 +272,8 @@ static long do_sys_truncate(const char _
goto put_write_and_out;
error = locks_verify_truncate(inode, NULL, length);
+ if (!error)
+ error = security_path_truncate(&path, length, 0, NULL);
if (!error) {
DQUOT_INIT(inode);
error = do_truncate(path.dentry, length, 0, NULL);
@@ -329,6 +331,9 @@ static long do_sys_ftruncate(unsigned in
error = locks_verify_truncate(inode, file, length);
if (!error)
+ error = security_path_truncate(&file->f_path, length,
+ ATTR_MTIME|ATTR_CTIME, file);
+ if (!error)
error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, file);
out_putf:
fput(file);
--- linux-2.6.27-rc7-mm1.orig/include/linux/security.h
+++ linux-2.6.27-rc7-mm1/include/linux/security.h
@@ -331,17 +331,37 @@ static inline void security_free_mnt_opt
* @dir contains the inode structure of the parent directory of the new link.
* @new_dentry contains the dentry structure for the new link.
* Return 0 if permission is granted.
+ * @path_link:
+ * Check permission before creating a new hard link to a file.
+ * @old_dentry contains the dentry structure for an existing link
+ * to the file.
+ * @new_dir contains the path structure of the parent directory of
+ * the new link.
+ * @new_dentry contains the dentry structure for the new link.
+ * Return 0 if permission is granted.
* @inode_unlink:
* Check the permission to remove a hard link to a file.
* @dir contains the inode structure of parent directory of the file.
* @dentry contains the dentry structure for file to be unlinked.
* Return 0 if permission is granted.
+ * @path_unlink:
+ * Check the permission to remove a hard link to a file.
+ * @dir contains the path structure of parent directory of the file.
+ * @dentry contains the dentry structure for file to be unlinked.
+ * Return 0 if permission is granted.
* @inode_symlink:
* Check the permission to create a symbolic link to a file.
* @dir contains the inode structure of parent directory of the symbolic link.
* @dentry contains the dentry structure of the symbolic link.
* @old_name contains the pathname of file.
* Return 0 if permission is granted.
+ * @path_symlink:
+ * Check the permission to create a symbolic link to a file.
+ * @dir contains the path structure of parent directory of
+ * the symbolic link.
+ * @dentry contains the dentry structure of the symbolic link.
+ * @old_name contains the pathname of file.
+ * Return 0 if permission is granted.
* @inode_mkdir:
* Check permissions to create a new directory in the existing directory
* associated with inode strcture @dir.
@@ -349,11 +369,25 @@ static inline void security_free_mnt_opt
* @dentry contains the dentry structure of new directory.
* @mode contains the mode of new directory.
* Return 0 if permission is granted.
+ * @path_mkdir:
+ * Check permissions to create a new directory in the existing directory
+ * associated with path strcture @path.
+ * @dir containst the path structure of parent of the directory
+ * to be created.
+ * @dentry contains the dentry structure of new directory.
+ * @mode contains the mode of new directory.
+ * Return 0 if permission is granted.
* @inode_rmdir:
* Check the permission to remove a directory.
* @dir contains the inode structure of parent of the directory to be removed.
* @dentry contains the dentry structure of directory to be removed.
* Return 0 if permission is granted.
+ * @path_rmdir:
+ * Check the permission to remove a directory.
+ * @dir contains the path structure of parent of the directory to be
+ * removed.
+ * @dentry contains the dentry structure of directory to be removed.
+ * Return 0 if permission is granted.
* @inode_mknod:
* Check permissions when creating a special file (or a socket or a fifo
* file created via the mknod system call). Note that if mknod operation
@@ -364,6 +398,15 @@ static inline void security_free_mnt_opt
* @mode contains the mode of the new file.
* @dev contains the device number.
* Return 0 if permission is granted.
+ * @path_mknod:
+ * Check permissions when creating a file. Note that this hook is called
+ * even if mknod operation is being done for a regular file.
+ * @dir contains the path structure of parent of the new file.
+ * @dentry contains the dentry structure of the new file.
+ * @mode contains the mode of the new file.
+ * @dev contains the undecoded device number. Use new_decode_dev() to get
+ * the decoded device number.
+ * Return 0 if permission is granted.
* @inode_rename:
* Check for permission to rename a file or directory.
* @old_dir contains the inode structure for parent of the old link.
@@ -371,6 +414,13 @@ static inline void security_free_mnt_opt
* @new_dir contains the inode structure for parent of the new link.
* @new_dentry contains the dentry structure of the new link.
* Return 0 if permission is granted.
+ * @path_rename:
+ * Check for permission to rename a file or directory.
+ * @old_dir contains the path structure for parent of the old link.
+ * @old_dentry contains the dentry structure of the old link.
+ * @new_dir contains the path structure for parent of the new link.
+ * @new_dentry contains the dentry structure of the new link.
+ * Return 0 if permission is granted.
* @inode_readlink:
* Check the permission to read the symbolic link.
* @dentry contains the dentry structure for the file link.
@@ -399,6 +449,13 @@ static inline void security_free_mnt_opt
* @dentry contains the dentry structure for the file.
* @attr is the iattr structure containing the new file attributes.
* Return 0 if permission is granted.
+ * @path_truncate:
+ * Check permission before truncating a file.
+ * @path contains the path structure for the file.
+ * @length is the new length of the file.
+ * @time_attrs is the flags passed to do_truncate().
+ * @filp is the file structure (may be NULL).
+ * Return 0 if permission is granted.
* @inode_getattr:
* Check permission before obtaining file attributes.
* @mnt is the vfsmount where the dentry was looked up
@@ -1327,6 +1384,22 @@ struct security_operations {
struct super_block *newsb);
int (*sb_parse_opts_str) (char *options, struct security_mnt_opts *opts);
+#ifdef CONFIG_SECURITY_PATH
+ int (*path_unlink) (struct path *dir, struct dentry *dentry);
+ int (*path_mkdir) (struct path *dir, struct dentry *dentry, int mode);
+ int (*path_rmdir) (struct path *dir, struct dentry *dentry);
+ int (*path_mknod) (struct path *dir, struct dentry *dentry, int mode,
+ unsigned int dev);
+ int (*path_truncate) (struct path *path, loff_t length,
+ unsigned int time_attrs, struct file *filp);
+ int (*path_symlink) (struct path *dir, struct dentry *dentry,
+ const char *old_name);
+ int (*path_link) (struct dentry *old_dentry, struct path *new_dir,
+ struct dentry *new_dentry);
+ int (*path_rename) (struct path *old_dir, struct dentry *old_dentry,
+ struct path *new_dir, struct dentry *new_dentry);
+#endif
+
int (*inode_alloc_security) (struct inode *inode);
void (*inode_free_security) (struct inode *inode);
int (*inode_init_security) (struct inode *inode, struct inode *dir,
@@ -2685,6 +2758,72 @@ static inline void security_skb_classify
#endif /* CONFIG_SECURITY_NETWORK_XFRM */
+#ifdef CONFIG_SECURITY_PATH
+int security_path_unlink(struct path *dir, struct dentry *dentry);
+int security_path_mkdir(struct path *dir, struct dentry *dentry, int mode);
+int security_path_rmdir(struct path *dir, struct dentry *dentry);
+int security_path_mknod(struct path *dir, struct dentry *dentry, int mode,
+ unsigned int dev);
+int security_path_truncate(struct path *path, loff_t length,
+ unsigned int time_attrs, struct file *filp);
+int security_path_symlink(struct path *dir, struct dentry *dentry,
+ const char *old_name);
+int security_path_link(struct dentry *old_dentry, struct path *new_dir,
+ struct dentry *new_dentry);
+int security_path_rename(struct path *old_dir, struct dentry *old_dentry,
+ struct path *new_dir, struct dentry *new_dentry);
+#else /* CONFIG_SECURITY_PATH */
+static inline int security_path_unlink(struct path *dir, struct dentry *dentry)
+{
+ return 0;
+}
+
+static inline int security_path_mkdir(struct path *dir, struct dentry *dentry,
+ int mode)
+{
+ return 0;
+}
+
+static inline int security_path_rmdir(struct path *dir, struct dentry *dentry)
+{
+ return 0;
+}
+
+static inline int security_path_mknod(struct path *dir, struct dentry *dentry,
+ int mode, unsigned int dev)
+{
+ return 0;
+}
+
+static inline int security_path_truncate(struct path *path, loff_t length,
+ unsigned int time_attrs,
+ struct file *filp)
+{
+ return 0;
+}
+
+static inline int security_path_symlink(struct path *dir, struct dentry *dentry,
+ const char *old_name)
+{
+ return 0;
+}
+
+static inline int security_path_link(struct dentry *old_dentry,
+ struct path *new_dir,
+ struct dentry *new_dentry)
+{
+ return 0;
+}
+
+static inline int security_path_rename(struct path *old_dir,
+ struct dentry *old_dentry,
+ struct path *new_dir,
+ struct dentry *new_dentry)
+{
+ return 0;
+}
+#endif /* CONFIG_SECURITY_PATH */
+
#ifdef CONFIG_KEYS
#ifdef CONFIG_SECURITY
--- linux-2.6.27-rc7-mm1.orig/net/unix/af_unix.c
+++ linux-2.6.27-rc7-mm1/net/unix/af_unix.c
@@ -828,7 +828,11 @@ static int unix_bind(struct socket *sock
err = mnt_want_write(nd.path.mnt);
if (err)
goto out_mknod_dput;
+ err = security_path_mknod(&nd.path, dentry, mode, 0);
+ if (err)
+ goto out_mknod_drop_write;
err = vfs_mknod(nd.path.dentry->d_inode, dentry, mode, 0);
+out_mknod_drop_write:
mnt_drop_write(nd.path.mnt);
if (err)
goto out_mknod_dput;
--- linux-2.6.27-rc7-mm1.orig/security/capability.c
+++ linux-2.6.27-rc7-mm1/security/capability.c
@@ -263,6 +263,53 @@ static void cap_inode_getsecid(const str
*secid = 0;
}
+#ifdef CONFIG_SECURITY_PATH
+static int cap_path_mknod(struct path *dir, struct dentry *dentry, int mode,
+ unsigned int dev)
+{
+ return 0;
+}
+
+static int cap_path_mkdir(struct path *dir, struct dentry *dentry, int mode)
+{
+ return 0;
+}
+
+static int cap_path_rmdir(struct path *dir, struct dentry *dentry)
+{
+ return 0;
+}
+
+static int cap_path_unlink(struct path *dir, struct dentry *dentry)
+{
+ return 0;
+}
+
+static int cap_path_symlink(struct path *dir, struct dentry *dentry,
+ const char *old_name)
+{
+ return 0;
+}
+
+static int cap_path_link(struct dentry *old_dentry, struct path *new_dir,
+ struct dentry *new_dentry)
+{
+ return 0;
+}
+
+static int cap_path_rename(struct path *old_path, struct dentry *old_dentry,
+ struct path *new_path, struct dentry *new_dentry)
+{
+ return 0;
+}
+
+static int cap_path_truncate(struct path *path, loff_t length,
+ unsigned int time_attrs, struct file *filp)
+{
+ return 0;
+}
+#endif
+
static int cap_file_permission(struct file *file, int mask)
{
return 0;
@@ -883,6 +930,16 @@ void security_fixup_ops(struct security_
set_to_cap_if_null(ops, inode_setsecurity);
set_to_cap_if_null(ops, inode_listsecurity);
set_to_cap_if_null(ops, inode_getsecid);
+#ifdef CONFIG_SECURITY_PATH
+ set_to_cap_if_null(ops, path_mknod);
+ set_to_cap_if_null(ops, path_mkdir);
+ set_to_cap_if_null(ops, path_rmdir);
+ set_to_cap_if_null(ops, path_unlink);
+ set_to_cap_if_null(ops, path_symlink);
+ set_to_cap_if_null(ops, path_link);
+ set_to_cap_if_null(ops, path_rename);
+ set_to_cap_if_null(ops, path_truncate);
+#endif
set_to_cap_if_null(ops, file_permission);
set_to_cap_if_null(ops, file_alloc_security);
set_to_cap_if_null(ops, file_free_security);
--- linux-2.6.27-rc7-mm1.orig/security/security.c
+++ linux-2.6.27-rc7-mm1/security/security.c
@@ -341,6 +341,72 @@ int security_inode_init_security(struct
}
EXPORT_SYMBOL(security_inode_init_security);
+#ifdef CONFIG_SECURITY_PATH
+int security_path_mknod(struct path *path, struct dentry *dentry, int mode,
+ unsigned int dev)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_mknod(path, dentry, mode, dev);
+}
+EXPORT_SYMBOL(security_path_mknod);
+
+int security_path_mkdir(struct path *path, struct dentry *dentry, int mode)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_mkdir(path, dentry, mode);
+}
+
+int security_path_rmdir(struct path *path, struct dentry *dentry)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_rmdir(path, dentry);
+}
+
+int security_path_unlink(struct path *path, struct dentry *dentry)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_unlink(path, dentry);
+}
+
+int security_path_symlink(struct path *path, struct dentry *dentry,
+ const char *old_name)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_symlink(path, dentry, old_name);
+}
+
+int security_path_link(struct dentry *old_dentry, struct path *new_dir,
+ struct dentry *new_dentry)
+{
+ if (unlikely(IS_PRIVATE(old_dentry->d_inode)))
+ return 0;
+ return security_ops->path_link(old_dentry, new_dir, new_dentry);
+}
+
+int security_path_rename(struct path *old_dir, struct dentry *old_dentry,
+ struct path *new_dir, struct dentry *new_dentry)
+{
+ if (unlikely(IS_PRIVATE(old_dentry->d_inode) ||
+ (new_dentry->d_inode && IS_PRIVATE(new_dentry->d_inode))))
+ return 0;
+ return security_ops->path_rename(old_dir, old_dentry, new_dir,
+ new_dentry);
+}
+
+int security_path_truncate(struct path *path, loff_t length,
+ unsigned int time_attrs, struct file *filp)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_truncate(path, length, time_attrs, filp);
+}
+#endif
+
int security_inode_create(struct inode *dir, struct dentry *dentry, int mode)
{
if (unlikely(IS_PRIVATE(dir)))
--- linux-2.6.27-rc7-mm1.orig/security/Kconfig
+++ linux-2.6.27-rc7-mm1/security/Kconfig
@@ -81,6 +81,15 @@ config SECURITY_NETWORK_XFRM
IPSec.
If you are unsure how to answer this question, answer N.
+config SECURITY_PATH
+ bool "Security hooks for pathname based access control"
+ depends on SECURITY
+ help
+ This enables the security hooks for pathname based access control.
+ If enabled, a security module can use these hooks to
+ implement pathname based access controls.
+ If you are unsure how to answer this question, answer N.
+
config SECURITY_FILE_CAPABILITIES
bool "File POSIX Capabilities"
default n
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Introduce new LSM hooks where vfsmount is available.
2008-09-24 8:00 ` Kentaro Takeda
@ 2008-09-24 13:28 ` Serge E. Hallyn
2008-09-24 21:18 ` James Morris
0 siblings, 1 reply; 16+ messages in thread
From: Serge E. Hallyn @ 2008-09-24 13:28 UTC (permalink / raw)
To: Kentaro Takeda
Cc: James Morris, Andrew Morton, linux-security-module, linux-fsdevel,
linux-kernel, Toshiharu Harada
Quoting Kentaro Takeda (takedakn@nttdata.co.jp):
> James Morris wrote:
> > I don't see any technical errors in this patch.
> Thanks for your review.
>
> > If it is going to be merged, please make a new config option for
> > path-based hooks (similar to that for the network hooks), so they can be
> > compiled out.
> I see, here it is.
>
>
> ---
> Subject: introduce new LSM hooks where vfsmount is available.
>
> ----- What is this patch for? -----
>
> There are security_inode_*() LSM hooks for attribute-based MAC, but they are not
> suitable for pathname-based MAC because they don't receive "struct vfsmount"
> information.
>
> ----- How this patch was developed? -----
>
> Two pathname-based MACs, AppArmor and TOMOYO Linux, are trying to merge
> upstream. But because of "struct vfsmount" problem, they have been unable to
> merge upstream.
>
> Here are the list of approaches and the reasons of denial.
>
> (1) Not using LSM
> http://lwn.net/Articles/277833/
>
> This approach was rejected because security modules should use LSM because the
> whole idea behind LSM was to have a single set of hooks for all security
> modules; if every module now adds its own set of hooks, that purpose will have
> been defeated and the kernel will turn into a big mess of security hooks.
>
> (2) Retrieving "struct vfsmount" from "struct task_struct".
> http://lkml.org/lkml/2007/11/5/388
>
> Since "struct task_struct" contains list of "struct vfsmount",
> "struct vfsmount" which corresponds to "struct dentry" can be retrieved from
> the list unless "mount --bind" is used.
>
> This approach turned out to cause a critical problem that getting namespace_sem
> lock from security_inode_*() triggers AB-BA deadlock.
>
> (3) Adding "struct vfsmount" parameter to VFS helper functions.
> http://lkml.org/lkml/2008/5/29/207
>
> This approach adds "struct vfsmount" to VFS helper functions (e.g. vfs_mkdir()
> and vfs_symlink()) and LSM hooks inside VFS helper functions. This approach is
> helpful for not only AppArmor and TOMOYO Linux 2.x but also SELinux and
> auditing purpose, for this approach allows existent LSM users to use pathnames
> in their access control and audit logs.
>
> This approach was rejected by Al Viro, the VFS maintainer, because he thinks
> individual filesystem should remain "struct vfsmount"-unaware and VFS helper
> functions should not receive "struct vfsmount".
>
> Al Viro also suggested to move existing security_inode_*() to out of VFS
> helper functions so that security_inode_*() can receive "struct vfsmount"
> without modifying VFS helper functions, but this suggestion was opposed by
> Stephen Smalley because changing the order of permission checks (i.e.
> MAC checks before DAC checks) is not acceptable.
>
> (4) Passing "struct vfsmount" via "struct task_struct".
> http://lkml.org/lkml/2007/11/16/157
>
> Since we didn't understand the reason why accessing "struct vfsmount" from
> LSM hooks inside VFS helper functions is not acceptable, we thought the reason
> why VFS helper functions don't receive "struct vfsmount" is the amount of
> modifications needed to do so. Thus, we proposed to pass "struct vfsmount" via
> "struct task_struct" so that modifications remain minimal.
>
> This approach was rejected because this is an abuse of "struct task_struct".
>
> (5) Remembering pathname of "struct vfsmount" via "struct task_struct".
> http://lkml.org/lkml/2008/8/19/16
>
> Since pathname of a "struct dentry" up to the mount point can be calculated
> without "struct vfsmount", absolute pathname of a "struct dentry" can be
> calculated if "struct task_struct" can remember absolute pathname of a
> "struct vfsmount" which corresponds to "struct dentry".
> As we now understand that Al Viro is opposing to access "struct vfsmount" from
> LSM hooks inside VFS helper functions, we gave up delivering "struct vfsmount"
> to LSM hooks inside VFS helper functions.
> Kernel 2.6.26 introduced read-only bind mount feature, and hooks for that
> feature (i.e. mnt_want_write() and mnt_drop_write()) were inserted around
> VFS helper functions call. Since mnt_want_write() receives "struct vfsmount"
> which corresponds to "struct dentry" that will be passed to subsequent VFS
> helper functions call, we associated pathname of "struct vfsmount" with
> "struct task_struct" instead of associating "struct vfsmount" itself.
>
> This approach was not explicitly rejected, but there seems to be performance
> problem.
>
> (6) Introducing new LSM hooks.
> (this patch)
>
> We understand that adding new LSM hooks which receive "struct vfsmount" outside
> VFS helper functions is the most straightforward approach. This approach has
> less impact to existing LSM module and no impact to VFS helper functions.
>
>
> Signed-off-by: Kentaro Takeda <takedakn@nttdata.co.jp>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Signed-off-by: Toshiharu Harada <haradats@nttdata.co.jp>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Crispin Cowan <crispin@crispincowan.com>
> Cc: Stephen Smalley <sds@tycho.nsa.gov>
> Cc: Casey Schaufler <casey@schaufler-ca.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> -----
>
> fs/namei.c | 37 ++++++++++++
> fs/open.c | 5 +
> include/linux/security.h | 139 +++++++++++++++++++++++++++++++++++++++++++++++
> net/unix/af_unix.c | 4 +
> security/Kconfig | 9 +++
> security/capability.c | 57 +++++++++++++++++++
> security/security.c | 66 ++++++++++++++++++++++
> 7 files changed, 317 insertions(+)
>
> --- linux-2.6.27-rc7-mm1.orig/fs/namei.c
> +++ linux-2.6.27-rc7-mm1/fs/namei.c
> @@ -1571,6 +1571,10 @@ int may_open(struct nameidata *nd, int a
> * Refuse to truncate files with mandatory locks held on them.
> */
> error = locks_verify_locked(inode);
> + if (!error)
> + error = security_path_truncate(&nd->path, 0,
> + ATTR_MTIME|ATTR_CTIME|ATTR_OPEN,
> + NULL);
> if (!error) {
> DQUOT_INIT(inode);
>
> @@ -1601,7 +1605,11 @@ static int __open_namei_create(struct na
>
> if (!IS_POSIXACL(dir->d_inode))
> mode &= ~current->fs->umask;
> + error = security_path_mknod(&nd->path, path->dentry, mode, 0);
> + if (error)
> + goto out_unlock;
> error = vfs_create(dir->d_inode, path->dentry, mode, nd);
> +out_unlock:
> mutex_unlock(&dir->d_inode->i_mutex);
> dput(nd->path.dentry);
> nd->path.dentry = path->dentry;
> @@ -2014,6 +2022,9 @@ asmlinkage long sys_mknodat(int dfd, con
> error = mnt_want_write(nd.path.mnt);
> if (error)
> goto out_dput;
> + error = security_path_mknod(&nd.path, dentry, mode, dev);
> + if (error)
> + goto out_drop_write;
> switch (mode & S_IFMT) {
> case 0: case S_IFREG:
> error = vfs_create(nd.path.dentry->d_inode,dentry,mode,&nd);
> @@ -2026,6 +2037,7 @@ asmlinkage long sys_mknodat(int dfd, con
> error = vfs_mknod(nd.path.dentry->d_inode,dentry,mode,0);
> break;
> }
> +out_drop_write:
> mnt_drop_write(nd.path.mnt);
> out_dput:
> dput(dentry);
> @@ -2085,7 +2097,11 @@ asmlinkage long sys_mkdirat(int dfd, con
> error = mnt_want_write(nd.path.mnt);
> if (error)
> goto out_dput;
> + error = security_path_mkdir(&nd.path, dentry, mode);
> + if (error)
> + goto out_drop_write;
> error = vfs_mkdir(nd.path.dentry->d_inode, dentry, mode);
> +out_drop_write:
> mnt_drop_write(nd.path.mnt);
> out_dput:
> dput(dentry);
> @@ -2192,7 +2208,11 @@ static long do_rmdir(int dfd, const char
> error = mnt_want_write(nd.path.mnt);
> if (error)
> goto exit3;
> + error = security_path_rmdir(&nd.path, dentry);
> + if (error)
> + goto exit4;
> error = vfs_rmdir(nd.path.dentry->d_inode, dentry);
> +exit4:
> mnt_drop_write(nd.path.mnt);
> exit3:
> dput(dentry);
> @@ -2274,7 +2294,11 @@ static long do_unlinkat(int dfd, const c
> error = mnt_want_write(nd.path.mnt);
> if (error)
> goto exit2;
> + error = security_path_unlink(&nd.path, dentry);
> + if (error)
> + goto exit3;
> error = vfs_unlink(nd.path.dentry->d_inode, dentry);
> +exit3:
> mnt_drop_write(nd.path.mnt);
> exit2:
> dput(dentry);
> @@ -2355,7 +2379,11 @@ asmlinkage long sys_symlinkat(const char
> error = mnt_want_write(nd.path.mnt);
> if (error)
> goto out_dput;
> + error = security_path_symlink(&nd.path, dentry, from);
> + if (error)
> + goto out_drop_write;
> error = vfs_symlink(nd.path.dentry->d_inode, dentry, from);
> +out_drop_write:
> mnt_drop_write(nd.path.mnt);
> out_dput:
> dput(dentry);
> @@ -2452,7 +2480,11 @@ asmlinkage long sys_linkat(int olddfd, c
> error = mnt_want_write(nd.path.mnt);
> if (error)
> goto out_dput;
> + error = security_path_link(old_path.dentry, &nd.path, new_dentry);
> + if (error)
> + goto out_drop_write;
> error = vfs_link(old_path.dentry, nd.path.dentry->d_inode, new_dentry);
> +out_drop_write:
> mnt_drop_write(nd.path.mnt);
> out_dput:
> dput(new_dentry);
> @@ -2682,8 +2714,13 @@ asmlinkage long sys_renameat(int olddfd,
> error = mnt_want_write(oldnd.path.mnt);
> if (error)
> goto exit5;
> + error = security_path_rename(&oldnd.path, old_dentry,
> + &newnd.path, new_dentry);
> + if (error)
> + goto exit6;
> error = vfs_rename(old_dir->d_inode, old_dentry,
> new_dir->d_inode, new_dentry);
> +exit6:
> mnt_drop_write(oldnd.path.mnt);
> exit5:
> dput(new_dentry);
> --- linux-2.6.27-rc7-mm1.orig/fs/open.c
> +++ linux-2.6.27-rc7-mm1/fs/open.c
> @@ -272,6 +272,8 @@ static long do_sys_truncate(const char _
> goto put_write_and_out;
>
> error = locks_verify_truncate(inode, NULL, length);
> + if (!error)
> + error = security_path_truncate(&path, length, 0, NULL);
> if (!error) {
> DQUOT_INIT(inode);
> error = do_truncate(path.dentry, length, 0, NULL);
> @@ -329,6 +331,9 @@ static long do_sys_ftruncate(unsigned in
>
> error = locks_verify_truncate(inode, file, length);
> if (!error)
> + error = security_path_truncate(&file->f_path, length,
> + ATTR_MTIME|ATTR_CTIME, file);
> + if (!error)
> error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, file);
> out_putf:
> fput(file);
> --- linux-2.6.27-rc7-mm1.orig/include/linux/security.h
> +++ linux-2.6.27-rc7-mm1/include/linux/security.h
> @@ -331,17 +331,37 @@ static inline void security_free_mnt_opt
> * @dir contains the inode structure of the parent directory of the new link.
> * @new_dentry contains the dentry structure for the new link.
> * Return 0 if permission is granted.
> + * @path_link:
> + * Check permission before creating a new hard link to a file.
> + * @old_dentry contains the dentry structure for an existing link
> + * to the file.
> + * @new_dir contains the path structure of the parent directory of
> + * the new link.
> + * @new_dentry contains the dentry structure for the new link.
> + * Return 0 if permission is granted.
> * @inode_unlink:
> * Check the permission to remove a hard link to a file.
> * @dir contains the inode structure of parent directory of the file.
> * @dentry contains the dentry structure for file to be unlinked.
> * Return 0 if permission is granted.
> + * @path_unlink:
> + * Check the permission to remove a hard link to a file.
> + * @dir contains the path structure of parent directory of the file.
> + * @dentry contains the dentry structure for file to be unlinked.
> + * Return 0 if permission is granted.
> * @inode_symlink:
> * Check the permission to create a symbolic link to a file.
> * @dir contains the inode structure of parent directory of the symbolic link.
> * @dentry contains the dentry structure of the symbolic link.
> * @old_name contains the pathname of file.
> * Return 0 if permission is granted.
> + * @path_symlink:
> + * Check the permission to create a symbolic link to a file.
> + * @dir contains the path structure of parent directory of
> + * the symbolic link.
> + * @dentry contains the dentry structure of the symbolic link.
> + * @old_name contains the pathname of file.
> + * Return 0 if permission is granted.
> * @inode_mkdir:
> * Check permissions to create a new directory in the existing directory
> * associated with inode strcture @dir.
> @@ -349,11 +369,25 @@ static inline void security_free_mnt_opt
> * @dentry contains the dentry structure of new directory.
> * @mode contains the mode of new directory.
> * Return 0 if permission is granted.
> + * @path_mkdir:
> + * Check permissions to create a new directory in the existing directory
> + * associated with path strcture @path.
> + * @dir containst the path structure of parent of the directory
> + * to be created.
> + * @dentry contains the dentry structure of new directory.
> + * @mode contains the mode of new directory.
> + * Return 0 if permission is granted.
> * @inode_rmdir:
> * Check the permission to remove a directory.
> * @dir contains the inode structure of parent of the directory to be removed.
> * @dentry contains the dentry structure of directory to be removed.
> * Return 0 if permission is granted.
> + * @path_rmdir:
> + * Check the permission to remove a directory.
> + * @dir contains the path structure of parent of the directory to be
> + * removed.
> + * @dentry contains the dentry structure of directory to be removed.
> + * Return 0 if permission is granted.
> * @inode_mknod:
> * Check permissions when creating a special file (or a socket or a fifo
> * file created via the mknod system call). Note that if mknod operation
> @@ -364,6 +398,15 @@ static inline void security_free_mnt_opt
> * @mode contains the mode of the new file.
> * @dev contains the device number.
> * Return 0 if permission is granted.
> + * @path_mknod:
> + * Check permissions when creating a file. Note that this hook is called
> + * even if mknod operation is being done for a regular file.
> + * @dir contains the path structure of parent of the new file.
> + * @dentry contains the dentry structure of the new file.
> + * @mode contains the mode of the new file.
> + * @dev contains the undecoded device number. Use new_decode_dev() to get
> + * the decoded device number.
> + * Return 0 if permission is granted.
> * @inode_rename:
> * Check for permission to rename a file or directory.
> * @old_dir contains the inode structure for parent of the old link.
> @@ -371,6 +414,13 @@ static inline void security_free_mnt_opt
> * @new_dir contains the inode structure for parent of the new link.
> * @new_dentry contains the dentry structure of the new link.
> * Return 0 if permission is granted.
> + * @path_rename:
> + * Check for permission to rename a file or directory.
> + * @old_dir contains the path structure for parent of the old link.
> + * @old_dentry contains the dentry structure of the old link.
> + * @new_dir contains the path structure for parent of the new link.
> + * @new_dentry contains the dentry structure of the new link.
> + * Return 0 if permission is granted.
> * @inode_readlink:
> * Check the permission to read the symbolic link.
> * @dentry contains the dentry structure for the file link.
> @@ -399,6 +449,13 @@ static inline void security_free_mnt_opt
> * @dentry contains the dentry structure for the file.
> * @attr is the iattr structure containing the new file attributes.
> * Return 0 if permission is granted.
> + * @path_truncate:
> + * Check permission before truncating a file.
> + * @path contains the path structure for the file.
> + * @length is the new length of the file.
> + * @time_attrs is the flags passed to do_truncate().
> + * @filp is the file structure (may be NULL).
> + * Return 0 if permission is granted.
> * @inode_getattr:
> * Check permission before obtaining file attributes.
> * @mnt is the vfsmount where the dentry was looked up
> @@ -1327,6 +1384,22 @@ struct security_operations {
> struct super_block *newsb);
> int (*sb_parse_opts_str) (char *options, struct security_mnt_opts *opts);
>
> +#ifdef CONFIG_SECURITY_PATH
> + int (*path_unlink) (struct path *dir, struct dentry *dentry);
> + int (*path_mkdir) (struct path *dir, struct dentry *dentry, int mode);
> + int (*path_rmdir) (struct path *dir, struct dentry *dentry);
> + int (*path_mknod) (struct path *dir, struct dentry *dentry, int mode,
> + unsigned int dev);
> + int (*path_truncate) (struct path *path, loff_t length,
> + unsigned int time_attrs, struct file *filp);
> + int (*path_symlink) (struct path *dir, struct dentry *dentry,
> + const char *old_name);
> + int (*path_link) (struct dentry *old_dentry, struct path *new_dir,
> + struct dentry *new_dentry);
> + int (*path_rename) (struct path *old_dir, struct dentry *old_dentry,
> + struct path *new_dir, struct dentry *new_dentry);
> +#endif
> +
> int (*inode_alloc_security) (struct inode *inode);
> void (*inode_free_security) (struct inode *inode);
> int (*inode_init_security) (struct inode *inode, struct inode *dir,
> @@ -2685,6 +2758,72 @@ static inline void security_skb_classify
>
> #endif /* CONFIG_SECURITY_NETWORK_XFRM */
>
> +#ifdef CONFIG_SECURITY_PATH
> +int security_path_unlink(struct path *dir, struct dentry *dentry);
> +int security_path_mkdir(struct path *dir, struct dentry *dentry, int mode);
> +int security_path_rmdir(struct path *dir, struct dentry *dentry);
> +int security_path_mknod(struct path *dir, struct dentry *dentry, int mode,
> + unsigned int dev);
> +int security_path_truncate(struct path *path, loff_t length,
> + unsigned int time_attrs, struct file *filp);
> +int security_path_symlink(struct path *dir, struct dentry *dentry,
> + const char *old_name);
> +int security_path_link(struct dentry *old_dentry, struct path *new_dir,
> + struct dentry *new_dentry);
> +int security_path_rename(struct path *old_dir, struct dentry *old_dentry,
> + struct path *new_dir, struct dentry *new_dentry);
> +#else /* CONFIG_SECURITY_PATH */
> +static inline int security_path_unlink(struct path *dir, struct dentry *dentry)
> +{
> + return 0;
> +}
> +
> +static inline int security_path_mkdir(struct path *dir, struct dentry *dentry,
> + int mode)
> +{
> + return 0;
> +}
> +
> +static inline int security_path_rmdir(struct path *dir, struct dentry *dentry)
> +{
> + return 0;
> +}
> +
> +static inline int security_path_mknod(struct path *dir, struct dentry *dentry,
> + int mode, unsigned int dev)
> +{
> + return 0;
> +}
> +
> +static inline int security_path_truncate(struct path *path, loff_t length,
> + unsigned int time_attrs,
> + struct file *filp)
> +{
> + return 0;
> +}
> +
> +static inline int security_path_symlink(struct path *dir, struct dentry *dentry,
> + const char *old_name)
> +{
> + return 0;
> +}
> +
> +static inline int security_path_link(struct dentry *old_dentry,
> + struct path *new_dir,
> + struct dentry *new_dentry)
> +{
> + return 0;
> +}
> +
> +static inline int security_path_rename(struct path *old_dir,
> + struct dentry *old_dentry,
> + struct path *new_dir,
> + struct dentry *new_dentry)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_SECURITY_PATH */
> +
> #ifdef CONFIG_KEYS
> #ifdef CONFIG_SECURITY
>
> --- linux-2.6.27-rc7-mm1.orig/net/unix/af_unix.c
> +++ linux-2.6.27-rc7-mm1/net/unix/af_unix.c
> @@ -828,7 +828,11 @@ static int unix_bind(struct socket *sock
> err = mnt_want_write(nd.path.mnt);
> if (err)
> goto out_mknod_dput;
> + err = security_path_mknod(&nd.path, dentry, mode, 0);
> + if (err)
> + goto out_mknod_drop_write;
> err = vfs_mknod(nd.path.dentry->d_inode, dentry, mode, 0);
> +out_mknod_drop_write:
> mnt_drop_write(nd.path.mnt);
> if (err)
> goto out_mknod_dput;
> --- linux-2.6.27-rc7-mm1.orig/security/capability.c
> +++ linux-2.6.27-rc7-mm1/security/capability.c
> @@ -263,6 +263,53 @@ static void cap_inode_getsecid(const str
> *secid = 0;
> }
>
> +#ifdef CONFIG_SECURITY_PATH
> +static int cap_path_mknod(struct path *dir, struct dentry *dentry, int mode,
> + unsigned int dev)
> +{
> + return 0;
> +}
> +
> +static int cap_path_mkdir(struct path *dir, struct dentry *dentry, int mode)
> +{
> + return 0;
> +}
> +
> +static int cap_path_rmdir(struct path *dir, struct dentry *dentry)
> +{
> + return 0;
> +}
> +
> +static int cap_path_unlink(struct path *dir, struct dentry *dentry)
> +{
> + return 0;
> +}
> +
> +static int cap_path_symlink(struct path *dir, struct dentry *dentry,
> + const char *old_name)
> +{
> + return 0;
> +}
> +
> +static int cap_path_link(struct dentry *old_dentry, struct path *new_dir,
> + struct dentry *new_dentry)
> +{
> + return 0;
> +}
> +
> +static int cap_path_rename(struct path *old_path, struct dentry *old_dentry,
> + struct path *new_path, struct dentry *new_dentry)
> +{
> + return 0;
> +}
> +
> +static int cap_path_truncate(struct path *path, loff_t length,
> + unsigned int time_attrs, struct file *filp)
> +{
> + return 0;
> +}
> +#endif
> +
> static int cap_file_permission(struct file *file, int mask)
> {
> return 0;
> @@ -883,6 +930,16 @@ void security_fixup_ops(struct security_
> set_to_cap_if_null(ops, inode_setsecurity);
> set_to_cap_if_null(ops, inode_listsecurity);
> set_to_cap_if_null(ops, inode_getsecid);
> +#ifdef CONFIG_SECURITY_PATH
> + set_to_cap_if_null(ops, path_mknod);
> + set_to_cap_if_null(ops, path_mkdir);
> + set_to_cap_if_null(ops, path_rmdir);
> + set_to_cap_if_null(ops, path_unlink);
> + set_to_cap_if_null(ops, path_symlink);
> + set_to_cap_if_null(ops, path_link);
> + set_to_cap_if_null(ops, path_rename);
> + set_to_cap_if_null(ops, path_truncate);
> +#endif
> set_to_cap_if_null(ops, file_permission);
> set_to_cap_if_null(ops, file_alloc_security);
> set_to_cap_if_null(ops, file_free_security);
> --- linux-2.6.27-rc7-mm1.orig/security/security.c
> +++ linux-2.6.27-rc7-mm1/security/security.c
> @@ -341,6 +341,72 @@ int security_inode_init_security(struct
> }
> EXPORT_SYMBOL(security_inode_init_security);
>
> +#ifdef CONFIG_SECURITY_PATH
> +int security_path_mknod(struct path *path, struct dentry *dentry, int mode,
> + unsigned int dev)
> +{
> + if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
> + return 0;
> + return security_ops->path_mknod(path, dentry, mode, dev);
> +}
> +EXPORT_SYMBOL(security_path_mknod);
> +
> +int security_path_mkdir(struct path *path, struct dentry *dentry, int mode)
> +{
> + if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
> + return 0;
> + return security_ops->path_mkdir(path, dentry, mode);
> +}
> +
> +int security_path_rmdir(struct path *path, struct dentry *dentry)
> +{
> + if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
> + return 0;
> + return security_ops->path_rmdir(path, dentry);
> +}
> +
> +int security_path_unlink(struct path *path, struct dentry *dentry)
> +{
> + if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
> + return 0;
> + return security_ops->path_unlink(path, dentry);
> +}
> +
> +int security_path_symlink(struct path *path, struct dentry *dentry,
> + const char *old_name)
> +{
> + if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
> + return 0;
> + return security_ops->path_symlink(path, dentry, old_name);
> +}
> +
> +int security_path_link(struct dentry *old_dentry, struct path *new_dir,
> + struct dentry *new_dentry)
> +{
> + if (unlikely(IS_PRIVATE(old_dentry->d_inode)))
> + return 0;
> + return security_ops->path_link(old_dentry, new_dir, new_dentry);
> +}
> +
> +int security_path_rename(struct path *old_dir, struct dentry *old_dentry,
> + struct path *new_dir, struct dentry *new_dentry)
> +{
> + if (unlikely(IS_PRIVATE(old_dentry->d_inode) ||
> + (new_dentry->d_inode && IS_PRIVATE(new_dentry->d_inode))))
> + return 0;
> + return security_ops->path_rename(old_dir, old_dentry, new_dir,
> + new_dentry);
> +}
> +
> +int security_path_truncate(struct path *path, loff_t length,
> + unsigned int time_attrs, struct file *filp)
> +{
> + if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
> + return 0;
> + return security_ops->path_truncate(path, length, time_attrs, filp);
> +}
> +#endif
> +
> int security_inode_create(struct inode *dir, struct dentry *dentry, int mode)
> {
> if (unlikely(IS_PRIVATE(dir)))
> --- linux-2.6.27-rc7-mm1.orig/security/Kconfig
> +++ linux-2.6.27-rc7-mm1/security/Kconfig
> @@ -81,6 +81,15 @@ config SECURITY_NETWORK_XFRM
> IPSec.
> If you are unsure how to answer this question, answer N.
>
> +config SECURITY_PATH
> + bool "Security hooks for pathname based access control"
> + depends on SECURITY
> + help
> + This enables the security hooks for pathname based access control.
> + If enabled, a security module can use these hooks to
> + implement pathname based access controls.
> + If you are unsure how to answer this question, answer N.
Is there any reason why users should have to deal with this option?
Would it make sense in this case to make the option and have tomoyo and
apparmor select it?
> config SECURITY_FILE_CAPABILITIES
> bool "File POSIX Capabilities"
> default n
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] Introduce new LSM hooks where vfsmount is available.
2008-09-24 13:28 ` Serge E. Hallyn
@ 2008-09-24 21:18 ` James Morris
0 siblings, 0 replies; 16+ messages in thread
From: James Morris @ 2008-09-24 21:18 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Kentaro Takeda, Andrew Morton, linux-security-module,
linux-fsdevel, linux-kernel, Toshiharu Harada
On Wed, 24 Sep 2008, Serge E. Hallyn wrote:
> Quoting Kentaro Takeda (takedakn@nttdata.co.jp):
[please trim quoted material..]
> >
> > +config SECURITY_PATH
> > + bool "Security hooks for pathname based access control"
> > + depends on SECURITY
> > + help
> > + This enables the security hooks for pathname based access control.
> > + If enabled, a security module can use these hooks to
> > + implement pathname based access controls.
> > + If you are unsure how to answer this question, answer N.
>
> Is there any reason why users should have to deal with this option?
> Would it make sense in this case to make the option and have tomoyo and
> apparmor select it?
Who are users in this case? It's probably useful as a separate option for
testing purposes and for people doing development. LSMs which depend on
it should probably still use SELECT in any case.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] introduce new LSM hooks where vfsmount is available.
@ 2008-12-17 4:24 Kentaro Takeda
2008-12-17 4:42 ` Al Viro
0 siblings, 1 reply; 16+ messages in thread
From: Kentaro Takeda @ 2008-12-17 4:24 UTC (permalink / raw)
To: viro
Cc: jmorris, akpm, linux-fsdevel, linux-security-module, linux-kernel,
haradats
Al, would you respond to this patch?
James, Andrew and we are waiting for your Acked-by: response.
Regards,
------- Forwarded Message
From: Al Viro <viro@ZenIV.linux.org.uk>
To: akpm@linux-foundation.org
Cc: linux-fsdevel@vger.kernel.org, takedakn@nttdata.co.jp,casey@schaufler-ca.com, crispin@crispincowan.com,haradats@nttdata.co.jp, hch@lst.de, jmorris@namei.org,penguin-kernel@I-love.SAKURA.ne.jp, sds@tycho.nsa.gov
Subject: Re: [patch 04/11] vfs: introduce new LSM hooks where vfsmount isavailable.
Date: Sat, 6 Dec 2008 07:10:09 +0000
On Mon, Dec 01, 2008 at 02:34:54PM -0800, akpm@linux-foundation.org wrote:
In principle, ACK.
> fs/namei.c | 37 +++++++++
> fs/open.c | 5 +
> include/linux/security.h | 139 +++++++++++++++++++++++++++++++++++++
> net/unix/af_unix.c | 4 +
> security/Kconfig | 9 ++
> security/capability.c | 57 +++++++++++++++
> security/security.c | 66 +++++++++++++++++
> 7 files changed, 317 insertions(+)
>
> diff -puN fs/namei.c~introduce-new-lsm-hooks-where-vfsmount-is-available fs/namei.c
> --- a/fs/namei.c~introduce-new-lsm-hooks-where-vfsmount-is-available
> +++ a/fs/namei.c
> @@ -1556,6 +1556,10 @@ int may_open(struct nameidata *nd, int a
> * Refuse to truncate files with mandatory locks held on them.
> */
> error = locks_verify_locked(inode);
> + if (!error)
> + error = security_path_truncate(&nd->path, 0,
> + ATTR_MTIME|ATTR_CTIME|ATTR_OPEN,
> + NULL);
Humm... What's the story with the last argument (struct file *) in there?
Explain that and you've got it merged; other than this part I have no
objections to that patch.
------- End of Forwarded Message
------- Forwarded Message
From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
To: viro@ZenIV.linux.org.uk
Cc: akpm@linux-foundation.org, linux-fsdevel@vger.kernel.org, takedakn@nttdata.co.jp, casey@schaufler-ca.com, crispin@crispincowan.com, haradats@nttdata.co.jp, hch@lst.de, jmorris@namei.org, sds@tycho.nsa.gov
Subject: Re: [patch 04/11] vfs: introduce new LSM hooks where vfsmount is available.
Date: Sat, 6 Dec 2008 21:24:31 +0900
Thank you very much for responding.
Al Viro wrote:
> On Mon, Dec 01, 2008 at 02:34:54PM -0800, akpm@linux-foundation.org wrote:
>
> In principle, ACK.
>
> > fs/namei.c | 37 +++++++++
> > fs/open.c | 5 +
> > include/linux/security.h | 139 +++++++++++++++++++++++++++++++++++++
> > net/unix/af_unix.c | 4 +
> > security/Kconfig | 9 ++
> > security/capability.c | 57 +++++++++++++++
> > security/security.c | 66 +++++++++++++++++
> > 7 files changed, 317 insertions(+)
> >
> > diff -puN fs/namei.c~introduce-new-lsm-hooks-where-vfsmount-is-available fs/namei.c
> > --- a/fs/namei.c~introduce-new-lsm-hooks-where-vfsmount-is-available
> > +++ a/fs/namei.c
> > @@ -1556,6 +1556,10 @@ int may_open(struct nameidata *nd, int a
> > * Refuse to truncate files with mandatory locks held on them.
> > */
> > error = locks_verify_locked(inode);
> > + if (!error)
> > + error = security_path_truncate(&nd->path, 0,
> > + ATTR_MTIME|ATTR_CTIME|ATTR_OPEN,
> > + NULL);
>
> Humm... What's the story with the last argument (struct file *) in there?
> Explain that and you've got it merged; other than this part I have no
> objections to that patch.
do_sys_ftruncate() calls do_truncate() with "struct file *", whereas
do_sys_truncate() and may_open() call do_truncate() with NULL.
TOMOYO is not using the last argument, but I'm passing all arguments without
modification so that a LSM module can know the last argument if that LSM module
needs the last argument.
Regards.
------- End of Forwarded Message
------- Forwarded Message
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: akpm@linux-foundation.org, linux-fsdevel@vger.kernel.org, takedakn@nttdata.co.jp, casey@schaufler-ca.com, crispin@crispincowan.com, haradats@nttdata.co.jp, hch@lst.de, jmorris@namei.org, sds@tycho.nsa.gov
Subject: Re: [patch 04/11] vfs: introduce new LSM hooks where vfsmount is available.
Date: Sat, 6 Dec 2008 18:29:02 +0000
On Sat, Dec 06, 2008 at 09:24:31PM +0900, Tetsuo Handa wrote:
> > Humm... What's the story with the last argument (struct file *) in there?
> > Explain that and you've got it merged; other than this part I have no
> > objections to that patch.
>
> do_sys_ftruncate() calls do_truncate() with "struct file *", whereas
> do_sys_truncate() and may_open() call do_truncate() with NULL.
> TOMOYO is not using the last argument, but I'm passing all arguments without
> modification so that a LSM module can know the last argument if that LSM module
> needs the last argument.
And if such a module ever shows up *and* its authors can give good reasons
for checks in that place that apply to ftruncate(2) but not to truncate(2),
we can always add such argument. Until then...
Note that similar story with struct nameidata * being passed to fs methods
had caused a lot of mess and we are still not through with it. Let's
stick to the things that make clear sense...
------- End of Forwarded Message
------- Forwarded Message
From: Kentaro Takeda <takedakn@nttdata.co.jp>
To: viro@ZenIV.linux.org.uk
Cc: penguin-kernel@i-love.sakura.ne.jp, akpm@linux-foundation.org,linux-fsdevel@vger.kernel.org, casey@schaufler-ca.com,crispin@crispincowan.com, haradats@nttdata.co.jp, hch@lst.de,jmorris@namei.org, sds@tycho.nsa.gov,linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [patch 04/11] vfs: introduce new LSM hooks where vfsmount isavailable.
Date: Mon, 08 Dec 2008 10:33:57 +0900
(added cc:lsm-list, linux-kernel-list)
Al Viro wrote:
> On Sat, Dec 06, 2008 at 09:24:31PM +0900, Tetsuo Handa wrote:
>
> > > Humm... What's the story with the last argument (struct file *) in there?
> > > Explain that and you've got it merged; other than this part I have no
> > > objections to that patch.
> >
> > do_sys_ftruncate() calls do_truncate() with "struct file *", whereas
> > do_sys_truncate() and may_open() call do_truncate() with NULL.
> > TOMOYO is not using the last argument, but I'm passing all arguments without
> > modification so that a LSM module can know the last argument if that LSM module
> > needs the last argument.
>
> And if such a module ever shows up *and* its authors can give good reasons
> for checks in that place that apply to ftruncate(2) but not to truncate(2),
> we can always add such argument. Until then...
>
> Note that similar story with struct nameidata * being passed to fs methods
> had caused a lot of mess and we are still not through with it. Let's
> stick to the things that make clear sense...
>
I see. I removed the last argument from this patch.
I attach the updated version of this patch with updated changelog.
I'm sorry for letting you repeat the same answer.
TOMOYO wants to do DAC checks before MAC checks. Thus, I tried to embed DAC's
code copied from VFS helper functions into security_path_*() hooks.
But I received a comment that duplicating DAC's code is ugly.
Thus, I was searching for a way to do MAC checks at security_inode_*() hooks.
To do MAC checks at security_inode_*() hooks, I need to somehow pass
vfsmount's pathname, but I now understood that passing something via implicit
arguments is not acceptable. Thus, I give up approaches which use implicit
arguments.
Regards,
(patch omitted)
------- End of Forwarded Message
------- Forwarded Message
From: Kentaro Takeda <takedakn@nttdata.co.jp>
To: viro@ZenIV.linux.org.uk
Cc: penguin-kernel@i-love.sakura.ne.jp, akpm@linux-foundation.org,linux-fsdevel@vger.kernel.org, casey@schaufler-ca.com,crispin@crispincowan.com, haradats@nttdata.co.jp, hch@lst.de,jmorris@namei.org, sds@tycho.nsa.gov,linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [patch 04/11] vfs: introduce new LSM hooks where vfsmount isavailable.
Date: Fri, 12 Dec 2008 10:59:39 +0900
Al, please give us Acked-by response.
(patch refreshed for mmotm-2008-12-11-15-20).
Regards,
---
Subject: vfs: introduce new LSM hooks where vfsmount is available.
----- What is this patch for? -----
There are security_inode_*() LSM hooks for attribute-based MAC, but they are
not suitable for pathname-based MAC because they don't receive
"struct vfsmount" information.
----- How this patch was developed? -----
Two pathname-based MACs, AppArmor and TOMOYO Linux, are trying to merge
upstream. But because of "struct vfsmount" problem, they have been unable to
merge upstream.
Here are the list of approaches and the reasons of denial.
(1) Not using LSM
http://lwn.net/Articles/277833/
This approach was rejected because security modules should use LSM because the
whole idea behind LSM was to have a single set of hooks for all security
modules; if every module now adds its own set of hooks, that purpose will have
been defeated and the kernel will turn into a big mess of security hooks.
(2) Retrieving "struct vfsmount" from "struct task_struct".
http://lkml.org/lkml/2007/11/5/388
Since "struct task_struct" contains list of "struct vfsmount",
"struct vfsmount" which corresponds to "struct dentry" can be retrieved from
the list unless "mount --bind" is used.
This approach turned out to cause a critical problem that getting
namespace_sem lock from security_inode_*() triggers AB-BA deadlock.
(3) Adding "struct vfsmount" parameter to VFS helper functions.
http://lkml.org/lkml/2008/5/29/207
This approach adds "struct vfsmount" to VFS helper functions (e.g. vfs_mkdir()
and vfs_symlink()) and LSM hooks inside VFS helper functions. This approach is
helpful for not only AppArmor and TOMOYO Linux 2.x but also SELinux and
auditing purpose, for this approach allows existent LSM users to use pathnames
in their access control and audit logs.
This approach was rejected by Al Viro, the VFS maintainer, because he thinks
individual filesystem should remain "struct vfsmount"-unaware and VFS helper
functions should not receive "struct vfsmount".
Al Viro also suggested to move existing security_inode_*() to out of VFS
helper functions so that security_inode_*() can receive "struct vfsmount"
without modifying VFS helper functions, but this suggestion was opposed by
Stephen Smalley because changing the order of permission checks (i.e.
MAC checks before DAC checks) is not acceptable.
(4) Passing "struct vfsmount" via "struct task_struct".
http://lkml.org/lkml/2007/11/16/157
Since we didn't understand the reason why accessing "struct vfsmount" from
LSM hooks inside VFS helper functions is not acceptable, we thought the reason
why VFS helper functions don't receive "struct vfsmount" is the amount of
modifications needed to do so. Thus, we proposed to pass "struct vfsmount" via
"struct task_struct" so that modifications remain minimal.
This approach was rejected because this is an abuse of "struct task_struct".
(5) Remembering pathname of "struct vfsmount" via "struct task_struct".
http://lkml.org/lkml/2008/8/19/16
Since pathname of a "struct dentry" up to the mount point can be calculated
without "struct vfsmount", absolute pathname of a "struct dentry" can be
calculated if "struct task_struct" can remember absolute pathname of a
"struct vfsmount" which corresponds to "struct dentry".
As we now understand that Al Viro is opposing to access "struct vfsmount" from
LSM hooks inside VFS helper functions, we gave up delivering "struct vfsmount"
itself to LSM hooks inside VFS helper functions.
Kernel 2.6.26 introduced read-only bind mount feature, and hooks for that
feature (i.e. mnt_want_write() and mnt_drop_write()) were inserted around
VFS helper functions call. Since mnt_want_write() receives "struct vfsmount"
which corresponds to "struct dentry" that will be passed to subsequent VFS
helper functions call, we associated pathname of "struct vfsmount" with
"struct task_struct" instead of associating "struct vfsmount" itself.
This approach was rejected because of the kludge and of having the effect of
vfs_mkdir() et.al. dependent on anything except the arguments it's getting.
(6) Introducing new LSM hooks where vfsmount is available.
(this patch)
We came to the conclusion that adding new LSM hooks which receive
all necessary arguments (e.g. "struct vfsmount" and "struct dentry")
outside VFS helper functions is the only acceptable approach.
This approach has less impact to existing LSM module and no impact to
VFS helper functions.
Signed-off-by: Kentaro Takeda <takedakn@nttdata.co.jp>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Signed-off-by: Toshiharu Harada <haradats@nttdata.co.jp>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Crispin Cowan <crispin@crispincowan.com>
Cc: Stephen Smalley <sds@tycho.nsa.gov>
Cc: Casey Schaufler <casey@schaufler-ca.com>
Cc: James Morris <jmorris@namei.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
fs/namei.c | 36 ++++++++++++
fs/open.c | 5 +
include/linux/security.h | 137 +++++++++++++++++++++++++++++++++++++++++++++++
net/unix/af_unix.c | 4 +
security/Kconfig | 9 +++
security/capability.c | 57 +++++++++++++++++++
security/security.c | 66 ++++++++++++++++++++++
7 files changed, 314 insertions(+)
--- linux-2.6.28-rc8-mm1.orig/fs/namei.c
+++ linux-2.6.28-rc8-mm1/fs/namei.c
@@ -1556,6 +1556,9 @@ int may_open(struct nameidata *nd, int a
* Refuse to truncate files with mandatory locks held on them.
*/
error = locks_verify_locked(inode);
+ if (!error)
+ error = security_path_truncate(&nd->path, 0,
+ ATTR_MTIME|ATTR_CTIME|ATTR_OPEN);
if (!error) {
DQUOT_INIT(inode);
@@ -1586,7 +1589,11 @@ static int __open_namei_create(struct na
if (!IS_POSIXACL(dir->d_inode))
mode &= ~current->fs->umask;
+ error = security_path_mknod(&nd->path, path->dentry, mode, 0);
+ if (error)
+ goto out_unlock;
error = vfs_create(dir->d_inode, path->dentry, mode, nd);
+out_unlock:
mutex_unlock(&dir->d_inode->i_mutex);
dput(nd->path.dentry);
nd->path.dentry = path->dentry;
@@ -1999,6 +2006,9 @@ asmlinkage long sys_mknodat(int dfd, con
error = mnt_want_write(nd.path.mnt);
if (error)
goto out_dput;
+ error = security_path_mknod(&nd.path, dentry, mode, dev);
+ if (error)
+ goto out_drop_write;
switch (mode & S_IFMT) {
case 0: case S_IFREG:
error = vfs_create(nd.path.dentry->d_inode,dentry,mode,&nd);
@@ -2011,6 +2021,7 @@ asmlinkage long sys_mknodat(int dfd, con
error = vfs_mknod(nd.path.dentry->d_inode,dentry,mode,0);
break;
}
+out_drop_write:
mnt_drop_write(nd.path.mnt);
out_dput:
dput(dentry);
@@ -2070,7 +2081,11 @@ asmlinkage long sys_mkdirat(int dfd, con
error = mnt_want_write(nd.path.mnt);
if (error)
goto out_dput;
+ error = security_path_mkdir(&nd.path, dentry, mode);
+ if (error)
+ goto out_drop_write;
error = vfs_mkdir(nd.path.dentry->d_inode, dentry, mode);
+out_drop_write:
mnt_drop_write(nd.path.mnt);
out_dput:
dput(dentry);
@@ -2180,7 +2195,11 @@ static long do_rmdir(int dfd, const char
error = mnt_want_write(nd.path.mnt);
if (error)
goto exit3;
+ error = security_path_rmdir(&nd.path, dentry);
+ if (error)
+ goto exit4;
error = vfs_rmdir(nd.path.dentry->d_inode, dentry);
+exit4:
mnt_drop_write(nd.path.mnt);
exit3:
dput(dentry);
@@ -2265,7 +2284,11 @@ static long do_unlinkat(int dfd, const c
error = mnt_want_write(nd.path.mnt);
if (error)
goto exit2;
+ error = security_path_unlink(&nd.path, dentry);
+ if (error)
+ goto exit3;
error = vfs_unlink(nd.path.dentry->d_inode, dentry);
+exit3:
mnt_drop_write(nd.path.mnt);
exit2:
dput(dentry);
@@ -2346,7 +2369,11 @@ asmlinkage long sys_symlinkat(const char
error = mnt_want_write(nd.path.mnt);
if (error)
goto out_dput;
+ error = security_path_symlink(&nd.path, dentry, from);
+ if (error)
+ goto out_drop_write;
error = vfs_symlink(nd.path.dentry->d_inode, dentry, from);
+out_drop_write:
mnt_drop_write(nd.path.mnt);
out_dput:
dput(dentry);
@@ -2443,7 +2470,11 @@ asmlinkage long sys_linkat(int olddfd, c
error = mnt_want_write(nd.path.mnt);
if (error)
goto out_dput;
+ error = security_path_link(old_path.dentry, &nd.path, new_dentry);
+ if (error)
+ goto out_drop_write;
error = vfs_link(old_path.dentry, nd.path.dentry->d_inode, new_dentry);
+out_drop_write:
mnt_drop_write(nd.path.mnt);
out_dput:
dput(new_dentry);
@@ -2677,8 +2708,13 @@ asmlinkage long sys_renameat(int olddfd,
error = mnt_want_write(oldnd.path.mnt);
if (error)
goto exit5;
+ error = security_path_rename(&oldnd.path, old_dentry,
+ &newnd.path, new_dentry);
+ if (error)
+ goto exit6;
error = vfs_rename(old_dir->d_inode, old_dentry,
new_dir->d_inode, new_dentry);
+exit6:
mnt_drop_write(oldnd.path.mnt);
exit5:
dput(new_dentry);
--- linux-2.6.28-rc8-mm1.orig/fs/open.c
+++ linux-2.6.28-rc8-mm1/fs/open.c
@@ -272,6 +272,8 @@ static long do_sys_truncate(const char _
goto put_write_and_out;
error = locks_verify_truncate(inode, NULL, length);
+ if (!error)
+ error = security_path_truncate(&path, length, 0);
if (!error) {
DQUOT_INIT(inode);
error = do_truncate(path.dentry, length, 0, NULL);
@@ -329,6 +331,9 @@ static long do_sys_ftruncate(unsigned in
error = locks_verify_truncate(inode, file, length);
if (!error)
+ error = security_path_truncate(&file->f_path, length,
+ ATTR_MTIME|ATTR_CTIME);
+ if (!error)
error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, file);
out_putf:
fput(file);
--- linux-2.6.28-rc8-mm1.orig/include/linux/security.h
+++ linux-2.6.28-rc8-mm1/include/linux/security.h
@@ -335,17 +335,37 @@ static inline void security_free_mnt_opt
* @dir contains the inode structure of the parent directory of the new link.
* @new_dentry contains the dentry structure for the new link.
* Return 0 if permission is granted.
+ * @path_link:
+ * Check permission before creating a new hard link to a file.
+ * @old_dentry contains the dentry structure for an existing link
+ * to the file.
+ * @new_dir contains the path structure of the parent directory of
+ * the new link.
+ * @new_dentry contains the dentry structure for the new link.
+ * Return 0 if permission is granted.
* @inode_unlink:
* Check the permission to remove a hard link to a file.
* @dir contains the inode structure of parent directory of the file.
* @dentry contains the dentry structure for file to be unlinked.
* Return 0 if permission is granted.
+ * @path_unlink:
+ * Check the permission to remove a hard link to a file.
+ * @dir contains the path structure of parent directory of the file.
+ * @dentry contains the dentry structure for file to be unlinked.
+ * Return 0 if permission is granted.
* @inode_symlink:
* Check the permission to create a symbolic link to a file.
* @dir contains the inode structure of parent directory of the symbolic link.
* @dentry contains the dentry structure of the symbolic link.
* @old_name contains the pathname of file.
* Return 0 if permission is granted.
+ * @path_symlink:
+ * Check the permission to create a symbolic link to a file.
+ * @dir contains the path structure of parent directory of
+ * the symbolic link.
+ * @dentry contains the dentry structure of the symbolic link.
+ * @old_name contains the pathname of file.
+ * Return 0 if permission is granted.
* @inode_mkdir:
* Check permissions to create a new directory in the existing directory
* associated with inode strcture @dir.
@@ -353,11 +373,25 @@ static inline void security_free_mnt_opt
* @dentry contains the dentry structure of new directory.
* @mode contains the mode of new directory.
* Return 0 if permission is granted.
+ * @path_mkdir:
+ * Check permissions to create a new directory in the existing directory
+ * associated with path strcture @path.
+ * @dir containst the path structure of parent of the directory
+ * to be created.
+ * @dentry contains the dentry structure of new directory.
+ * @mode contains the mode of new directory.
+ * Return 0 if permission is granted.
* @inode_rmdir:
* Check the permission to remove a directory.
* @dir contains the inode structure of parent of the directory to be removed.
* @dentry contains the dentry structure of directory to be removed.
* Return 0 if permission is granted.
+ * @path_rmdir:
+ * Check the permission to remove a directory.
+ * @dir contains the path structure of parent of the directory to be
+ * removed.
+ * @dentry contains the dentry structure of directory to be removed.
+ * Return 0 if permission is granted.
* @inode_mknod:
* Check permissions when creating a special file (or a socket or a fifo
* file created via the mknod system call). Note that if mknod operation
@@ -368,6 +402,15 @@ static inline void security_free_mnt_opt
* @mode contains the mode of the new file.
* @dev contains the device number.
* Return 0 if permission is granted.
+ * @path_mknod:
+ * Check permissions when creating a file. Note that this hook is called
+ * even if mknod operation is being done for a regular file.
+ * @dir contains the path structure of parent of the new file.
+ * @dentry contains the dentry structure of the new file.
+ * @mode contains the mode of the new file.
+ * @dev contains the undecoded device number. Use new_decode_dev() to get
+ * the decoded device number.
+ * Return 0 if permission is granted.
* @inode_rename:
* Check for permission to rename a file or directory.
* @old_dir contains the inode structure for parent of the old link.
@@ -375,6 +418,13 @@ static inline void security_free_mnt_opt
* @new_dir contains the inode structure for parent of the new link.
* @new_dentry contains the dentry structure of the new link.
* Return 0 if permission is granted.
+ * @path_rename:
+ * Check for permission to rename a file or directory.
+ * @old_dir contains the path structure for parent of the old link.
+ * @old_dentry contains the dentry structure of the old link.
+ * @new_dir contains the path structure for parent of the new link.
+ * @new_dentry contains the dentry structure of the new link.
+ * Return 0 if permission is granted.
* @inode_readlink:
* Check the permission to read the symbolic link.
* @dentry contains the dentry structure for the file link.
@@ -403,6 +453,12 @@ static inline void security_free_mnt_opt
* @dentry contains the dentry structure for the file.
* @attr is the iattr structure containing the new file attributes.
* Return 0 if permission is granted.
+ * @path_truncate:
+ * Check permission before truncating a file.
+ * @path contains the path structure for the file.
+ * @length is the new length of the file.
+ * @time_attrs is the flags passed to do_truncate().
+ * Return 0 if permission is granted.
* @inode_getattr:
* Check permission before obtaining file attributes.
* @mnt is the vfsmount where the dentry was looked up
@@ -1331,6 +1387,22 @@ struct security_operations {
struct super_block *newsb);
int (*sb_parse_opts_str) (char *options, struct security_mnt_opts *opts);
+#ifdef CONFIG_SECURITY_PATH
+ int (*path_unlink) (struct path *dir, struct dentry *dentry);
+ int (*path_mkdir) (struct path *dir, struct dentry *dentry, int mode);
+ int (*path_rmdir) (struct path *dir, struct dentry *dentry);
+ int (*path_mknod) (struct path *dir, struct dentry *dentry, int mode,
+ unsigned int dev);
+ int (*path_truncate) (struct path *path, loff_t length,
+ unsigned int time_attrs);
+ int (*path_symlink) (struct path *dir, struct dentry *dentry,
+ const char *old_name);
+ int (*path_link) (struct dentry *old_dentry, struct path *new_dir,
+ struct dentry *new_dentry);
+ int (*path_rename) (struct path *old_dir, struct dentry *old_dentry,
+ struct path *new_dir, struct dentry *new_dentry);
+#endif
+
int (*inode_alloc_security) (struct inode *inode);
void (*inode_free_security) (struct inode *inode);
int (*inode_init_security) (struct inode *inode, struct inode *dir,
@@ -2705,6 +2777,71 @@ static inline void security_skb_classify
#endif /* CONFIG_SECURITY_NETWORK_XFRM */
+#ifdef CONFIG_SECURITY_PATH
+int security_path_unlink(struct path *dir, struct dentry *dentry);
+int security_path_mkdir(struct path *dir, struct dentry *dentry, int mode);
+int security_path_rmdir(struct path *dir, struct dentry *dentry);
+int security_path_mknod(struct path *dir, struct dentry *dentry, int mode,
+ unsigned int dev);
+int security_path_truncate(struct path *path, loff_t length,
+ unsigned int time_attrs);
+int security_path_symlink(struct path *dir, struct dentry *dentry,
+ const char *old_name);
+int security_path_link(struct dentry *old_dentry, struct path *new_dir,
+ struct dentry *new_dentry);
+int security_path_rename(struct path *old_dir, struct dentry *old_dentry,
+ struct path *new_dir, struct dentry *new_dentry);
+#else /* CONFIG_SECURITY_PATH */
+static inline int security_path_unlink(struct path *dir, struct dentry *dentry)
+{
+ return 0;
+}
+
+static inline int security_path_mkdir(struct path *dir, struct dentry *dentry,
+ int mode)
+{
+ return 0;
+}
+
+static inline int security_path_rmdir(struct path *dir, struct dentry *dentry)
+{
+ return 0;
+}
+
+static inline int security_path_mknod(struct path *dir, struct dentry *dentry,
+ int mode, unsigned int dev)
+{
+ return 0;
+}
+
+static inline int security_path_truncate(struct path *path, loff_t length,
+ unsigned int time_attrs)
+{
+ return 0;
+}
+
+static inline int security_path_symlink(struct path *dir, struct dentry *dentry,
+ const char *old_name)
+{
+ return 0;
+}
+
+static inline int security_path_link(struct dentry *old_dentry,
+ struct path *new_dir,
+ struct dentry *new_dentry)
+{
+ return 0;
+}
+
+static inline int security_path_rename(struct path *old_dir,
+ struct dentry *old_dentry,
+ struct path *new_dir,
+ struct dentry *new_dentry)
+{
+ return 0;
+}
+#endif /* CONFIG_SECURITY_PATH */
+
#ifdef CONFIG_KEYS
#ifdef CONFIG_SECURITY
--- linux-2.6.28-rc8-mm1.orig/net/unix/af_unix.c
+++ linux-2.6.28-rc8-mm1/net/unix/af_unix.c
@@ -836,7 +836,11 @@ static int unix_bind(struct socket *sock
err = mnt_want_write(nd.path.mnt);
if (err)
goto out_mknod_dput;
+ err = security_path_mknod(&nd.path, dentry, mode, 0);
+ if (err)
+ goto out_mknod_drop_write;
err = vfs_mknod(nd.path.dentry->d_inode, dentry, mode, 0);
+out_mknod_drop_write:
mnt_drop_write(nd.path.mnt);
if (err)
goto out_mknod_dput;
--- linux-2.6.28-rc8-mm1.orig/security/Kconfig
+++ linux-2.6.28-rc8-mm1/security/Kconfig
@@ -81,6 +81,15 @@ config SECURITY_NETWORK_XFRM
IPSec.
If you are unsure how to answer this question, answer N.
+config SECURITY_PATH
+ bool "Security hooks for pathname based access control"
+ depends on SECURITY
+ help
+ This enables the security hooks for pathname based access control.
+ If enabled, a security module can use these hooks to
+ implement pathname based access controls.
+ If you are unsure how to answer this question, answer N.
+
config SECURITY_FILE_CAPABILITIES
bool "File POSIX Capabilities"
default n
--- linux-2.6.28-rc8-mm1.orig/security/capability.c
+++ linux-2.6.28-rc8-mm1/security/capability.c
@@ -263,6 +263,53 @@ static void cap_inode_getsecid(const str
*secid = 0;
}
+#ifdef CONFIG_SECURITY_PATH
+static int cap_path_mknod(struct path *dir, struct dentry *dentry, int mode,
+ unsigned int dev)
+{
+ return 0;
+}
+
+static int cap_path_mkdir(struct path *dir, struct dentry *dentry, int mode)
+{
+ return 0;
+}
+
+static int cap_path_rmdir(struct path *dir, struct dentry *dentry)
+{
+ return 0;
+}
+
+static int cap_path_unlink(struct path *dir, struct dentry *dentry)
+{
+ return 0;
+}
+
+static int cap_path_symlink(struct path *dir, struct dentry *dentry,
+ const char *old_name)
+{
+ return 0;
+}
+
+static int cap_path_link(struct dentry *old_dentry, struct path *new_dir,
+ struct dentry *new_dentry)
+{
+ return 0;
+}
+
+static int cap_path_rename(struct path *old_path, struct dentry *old_dentry,
+ struct path *new_path, struct dentry *new_dentry)
+{
+ return 0;
+}
+
+static int cap_path_truncate(struct path *path, loff_t length,
+ unsigned int time_attrs)
+{
+ return 0;
+}
+#endif
+
static int cap_file_permission(struct file *file, int mask)
{
return 0;
@@ -883,6 +930,16 @@ void security_fixup_ops(struct security_
set_to_cap_if_null(ops, inode_setsecurity);
set_to_cap_if_null(ops, inode_listsecurity);
set_to_cap_if_null(ops, inode_getsecid);
+#ifdef CONFIG_SECURITY_PATH
+ set_to_cap_if_null(ops, path_mknod);
+ set_to_cap_if_null(ops, path_mkdir);
+ set_to_cap_if_null(ops, path_rmdir);
+ set_to_cap_if_null(ops, path_unlink);
+ set_to_cap_if_null(ops, path_symlink);
+ set_to_cap_if_null(ops, path_link);
+ set_to_cap_if_null(ops, path_rename);
+ set_to_cap_if_null(ops, path_truncate);
+#endif
set_to_cap_if_null(ops, file_permission);
set_to_cap_if_null(ops, file_alloc_security);
set_to_cap_if_null(ops, file_free_security);
--- linux-2.6.28-rc8-mm1.orig/security/security.c
+++ linux-2.6.28-rc8-mm1/security/security.c
@@ -355,6 +355,72 @@ int security_inode_init_security(struct
}
EXPORT_SYMBOL(security_inode_init_security);
+#ifdef CONFIG_SECURITY_PATH
+int security_path_mknod(struct path *path, struct dentry *dentry, int mode,
+ unsigned int dev)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_mknod(path, dentry, mode, dev);
+}
+EXPORT_SYMBOL(security_path_mknod);
+
+int security_path_mkdir(struct path *path, struct dentry *dentry, int mode)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_mkdir(path, dentry, mode);
+}
+
+int security_path_rmdir(struct path *path, struct dentry *dentry)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_rmdir(path, dentry);
+}
+
+int security_path_unlink(struct path *path, struct dentry *dentry)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_unlink(path, dentry);
+}
+
+int security_path_symlink(struct path *path, struct dentry *dentry,
+ const char *old_name)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_symlink(path, dentry, old_name);
+}
+
+int security_path_link(struct dentry *old_dentry, struct path *new_dir,
+ struct dentry *new_dentry)
+{
+ if (unlikely(IS_PRIVATE(old_dentry->d_inode)))
+ return 0;
+ return security_ops->path_link(old_dentry, new_dir, new_dentry);
+}
+
+int security_path_rename(struct path *old_dir, struct dentry *old_dentry,
+ struct path *new_dir, struct dentry *new_dentry)
+{
+ if (unlikely(IS_PRIVATE(old_dentry->d_inode) ||
+ (new_dentry->d_inode && IS_PRIVATE(new_dentry->d_inode))))
+ return 0;
+ return security_ops->path_rename(old_dir, old_dentry, new_dir,
+ new_dentry);
+}
+
+int security_path_truncate(struct path *path, loff_t length,
+ unsigned int time_attrs)
+{
+ if (unlikely(IS_PRIVATE(path->dentry->d_inode)))
+ return 0;
+ return security_ops->path_truncate(path, length, time_attrs);
+}
+#endif
+
int security_inode_create(struct inode *dir, struct dentry *dentry, int mode)
{
if (unlikely(IS_PRIVATE(dir)))
------- End of Forwarded Message
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] introduce new LSM hooks where vfsmount is available.
2008-12-17 4:24 [PATCH] introduce new LSM hooks where vfsmount is available Kentaro Takeda
@ 2008-12-17 4:42 ` Al Viro
2008-12-17 4:50 ` Al Viro
0 siblings, 1 reply; 16+ messages in thread
From: Al Viro @ 2008-12-17 4:42 UTC (permalink / raw)
To: Kentaro Takeda
Cc: jmorris, akpm, linux-fsdevel, linux-security-module, linux-kernel,
haradats
On Wed, Dec 17, 2008 at 01:24:15PM +0900, Kentaro Takeda wrote:
> Al, would you respond to this patch?
> James, Andrew and we are waiting for your Acked-by: response.
In queue. FWIW, I'm about to post the current audit patchset to linux-audit,
push that crap to audit git and then I'm through with that turd. And
_finally_ can get to VFS queue sorting.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] introduce new LSM hooks where vfsmount is available.
2008-12-17 4:42 ` Al Viro
@ 2008-12-17 4:50 ` Al Viro
2008-12-17 5:06 ` Kentaro Takeda
0 siblings, 1 reply; 16+ messages in thread
From: Al Viro @ 2008-12-17 4:50 UTC (permalink / raw)
To: Kentaro Takeda
Cc: jmorris, akpm, linux-fsdevel, linux-security-module, linux-kernel,
haradats
On Wed, Dec 17, 2008 at 04:42:16AM +0000, Al Viro wrote:
> On Wed, Dec 17, 2008 at 01:24:15PM +0900, Kentaro Takeda wrote:
> > Al, would you respond to this patch?
> > James, Andrew and we are waiting for your Acked-by: response.
>
> In queue. FWIW, I'm about to post the current audit patchset to linux-audit,
> push that crap to audit git and then I'm through with that turd. And
> _finally_ can get to VFS queue sorting.
"That crap" == audit patches for .29-rc1, that is.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] introduce new LSM hooks where vfsmount is available.
2008-12-17 4:50 ` Al Viro
@ 2008-12-17 5:06 ` Kentaro Takeda
0 siblings, 0 replies; 16+ messages in thread
From: Kentaro Takeda @ 2008-12-17 5:06 UTC (permalink / raw)
To: viro
Cc: jmorris, akpm, linux-fsdevel, linux-security-module, linux-kernel,
haradats
Al Viro wrote:
> On Wed, Dec 17, 2008 at 04:42:16AM +0000, Al Viro wrote:
>> On Wed, Dec 17, 2008 at 01:24:15PM +0900, Kentaro Takeda wrote:
>>> Al, would you respond to this patch?
>>> James, Andrew and we are waiting for your Acked-by: response.
>> In queue. FWIW, I'm about to post the current audit patchset to linux-audit,
>> push that crap to audit git and then I'm through with that turd. And
>> _finally_ can get to VFS queue sorting.
>
> "That crap" == audit patches for .29-rc1, that is.
I see, thank you for informing us.
We'll wait for your work.
Regards,
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2008-12-17 5:06 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-12-17 4:24 [PATCH] introduce new LSM hooks where vfsmount is available Kentaro Takeda
2008-12-17 4:42 ` Al Viro
2008-12-17 4:50 ` Al Viro
2008-12-17 5:06 ` Kentaro Takeda
[not found] <48C65E9D.3070106@nttdata.co.jp>
[not found] ` <5fb14edc0809090902m32431bf6yf518a457e410764d@mail.gmail.com>
[not found] ` <20080910134540.45ec1272.akpm@linux-foundation.org>
[not found] ` <48C877DE.4040402@nttdata.co.jp>
2008-09-17 2:16 ` [PATCH] Introduce " Kentaro Takeda
2008-09-17 4:00 ` Casey Schaufler
2008-09-17 8:35 ` Kentaro Takeda
2008-09-17 17:46 ` david
2008-09-23 6:06 ` James Morris
2008-09-23 6:12 ` david
2008-09-23 15:02 ` Stephen Smalley
2008-09-23 15:20 ` Alexey Dobriyan
2008-09-23 21:20 ` James Morris
2008-09-24 8:00 ` Kentaro Takeda
2008-09-24 13:28 ` Serge E. Hallyn
2008-09-24 21:18 ` James Morris
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).