On Thu, 2008-05-29 at 15:49 +0200, Miklos Szeredi wrote:
> plain text document attachment (security_create_path.patch)
> From: Miklos Szeredi <[email protected]>
>
> In the inode_create() security operation and related functions pass
> the path (vfsmount + dentry) to the parent directory instead of the
> inode. AppArmor will need this.
>
> Signed-off-by: Miklos Szeredi <[email protected]>
> ---
> fs/namei.c | 12 ++++++------
> include/linux/security.h | 9 ++++-----
> security/dummy.c | 4 ++--
> security/security.c | 4 ++--
> security/selinux/hooks.c | 5 +++--
> 5 files changed, 17 insertions(+), 17 deletions(-)
>
> Index: linux-2.6/fs/namei.c
> ===================================================================
> --- linux-2.6.orig/fs/namei.c 2008-05-29 12:20:49.000000000 +0200
> +++ linux-2.6/fs/namei.c 2008-05-29 12:20:51.000000000 +0200
> @@ -1586,11 +1586,11 @@ void unlock_rename(struct dentry *p1, st
> }
> }
>
> -static int vfs_create(struct dentry *dir_dentry, struct dentry *dentry,
> +static int vfs_create(struct path *dir_path, struct dentry *dentry,
> int mode, struct nameidata *nd)
> {
> - struct inode *dir = dir_dentry->d_inode;
> - int error = may_create(dir_dentry, dentry);
> + struct inode *dir = dir_path->dentry->d_inode;
> + int error = may_create(dir_path->dentry, dentry);
>
> if (error)
> return error;
> @@ -1599,7 +1599,7 @@ static int vfs_create(struct dentry *dir
> return -EACCES; /* shouldn't it be ENOSYS? */
> mode &= S_IALLUGO;
> mode |= S_IFREG;
> - error = security_inode_create(dir, dentry, mode);
> + error = security_inode_create(dir_path, dentry, mode);
> if (error)
> return error;
> DQUOT_INIT(dir);
> @@ -1615,7 +1615,7 @@ int path_create(struct path *dir_path, s
> int error = mnt_want_write(dir_path->mnt);
>
> if (!error) {
> - error = vfs_create(dir_path->dentry, dentry, mode, nd);
> + error = vfs_create(dir_path, dentry, mode, nd);
> mnt_drop_write(dir_path->mnt);
> }
>
> @@ -1718,7 +1718,7 @@ static int __open_namei_create(struct na
>
> if (!IS_POSIXACL(dir->d_inode))
> mode &= ~current->fs->umask;
> - error = vfs_create(dir, path->dentry, mode, nd);
> + error = vfs_create(&nd->path, path->dentry, mode, nd);
> mutex_unlock(&dir->d_inode->i_mutex);
> dput(nd->path.dentry);
> nd->path.dentry = path->dentry;
> Index: linux-2.6/include/linux/security.h
> ===================================================================
> --- linux-2.6.orig/include/linux/security.h 2008-05-29 12:20:48.000000000 +0200
> +++ linux-2.6/include/linux/security.h 2008-05-29 12:20:51.000000000 +0200
> @@ -339,7 +339,7 @@ static inline void security_free_mnt_opt
> * -ENOMEM on memory allocation failure.
> * @inode_create:
> * Check permission to create a regular file.
> - * @dir contains inode structure of the parent of the new file.
> + * @dir contains the path to the parent of the new file.
> * @dentry contains the dentry structure for the file to be created.
> * @mode contains the file mode of the file to be created.
> * Return 0 if permission is granted.
> @@ -1353,8 +1353,7 @@ struct security_operations {
> void (*inode_free_security) (struct inode *inode);
> int (*inode_init_security) (struct inode *inode, struct inode *dir,
> char **name, void **value, size_t *len);
> - int (*inode_create) (struct inode *dir,
> - struct dentry *dentry, int mode);
> + int (*inode_create) (struct path *dir, struct dentry *dentry, int mode);
> int (*inode_link) (struct dentry *old_dentry,
> struct inode *dir, struct dentry *new_dentry);
> int (*inode_unlink) (struct inode *dir, struct dentry *dentry);
> @@ -1626,7 +1625,7 @@ int security_inode_alloc(struct inode *i
> void security_inode_free(struct inode *inode);
> int security_inode_init_security(struct inode *inode, struct inode *dir,
> char **name, void **value, size_t *len);
> -int security_inode_create(struct inode *dir, struct dentry *dentry, int mode);
> +int security_inode_create(struct path *dir, struct dentry *dentry, int mode);
> int security_inode_link(struct dentry *old_dentry, struct inode *dir,
> struct dentry *new_dentry);
> int security_inode_unlink(struct inode *dir, struct dentry *dentry);
> @@ -1964,7 +1963,7 @@ static inline int security_inode_init_se
> return -EOPNOTSUPP;
> }
>
> -static inline int security_inode_create(struct inode *dir,
> +static inline int security_inode_create(struct path *dir,
> struct dentry *dentry,
> int mode)
> {
> Index: linux-2.6/security/dummy.c
> ===================================================================
> --- linux-2.6.orig/security/dummy.c 2008-05-29 12:20:48.000000000 +0200
> +++ linux-2.6/security/dummy.c 2008-05-29 12:20:51.000000000 +0200
> @@ -286,8 +286,8 @@ static int dummy_inode_init_security (st
> return -EOPNOTSUPP;
> }
>
> -static int dummy_inode_create (struct inode *inode, struct dentry *dentry,
> - int mask)
> +static int dummy_inode_create(struct path *dir, struct dentry *dentry,
> + int mask)
> {
> return 0;
> }
> Index: linux-2.6/security/selinux/hooks.c
> ===================================================================
> --- linux-2.6.orig/security/selinux/hooks.c 2008-05-29 12:20:48.000000000 +0200
> +++ linux-2.6/security/selinux/hooks.c 2008-05-29 12:20:51.000000000 +0200
> @@ -2482,9 +2482,10 @@ static int selinux_inode_init_security(s
> return 0;
> }
>
> -static int selinux_inode_create(struct inode *dir, struct dentry *dentry, int mask)
> +static int selinux_inode_create(struct path *dir, struct dentry *dentry,
> + int mask)
> {
> - return may_create(dir, dentry, SECCLASS_FILE);
> + return may_create(dir->dentry->d_inode, dentry, SECCLASS_FILE);
> }
>
> static int selinux_inode_link(struct dentry *old_dentry, struct inode *dir, struct dentry *new_dentry)
This may be moot given the vfs maintainers' objections, but if you were
to make this change, then logically you'd push the struct path all the
way down and set it in the avc_audit_data so that it could be used by
avc_audit() for emitting a pathname in the audit record. Likewise for
the other hook changes.
--
Stephen Smalley
National Security Agency
Stephen Smalley wrote:
> This may be moot given the vfs maintainers' objections, but if you were
> to make this change, then logically you'd push the struct path all the
> way down and set it in the avc_audit_data so that it could be used by
> avc_audit() for emitting a pathname in the audit record. Likewise for
> the other hook changes.
Yes. That's one of improvements made possible by Miklos's patches.
----------
Subject: SELINUX: Set vfsmount field for audit logs.
By applying Miklos's patches which pass "struct vfsmount" to LSM
(posted at http://lkml.org/lkml/2008/5/29/207 ),
SELinux's audit logs can generate absolute pathnames for more operations.
Signed-off-by: Tetsuo Handa <[email protected]>
---
security/selinux/hooks.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
--- vfs.orig/security/selinux/hooks.c
+++ vfs/security/selinux/hooks.c
@@ -1427,10 +1427,11 @@ static int file_has_perm(struct task_str
}
/* Check whether a task can create a file. */
-static int may_create(struct inode *dir,
+static int may_create(struct path *dir_path,
struct dentry *dentry,
u16 tclass)
{
+ struct inode *dir = dir_path->dentry->d_inode;
struct task_security_struct *tsec;
struct inode_security_struct *dsec;
struct superblock_security_struct *sbsec;
@@ -1443,6 +1444,7 @@ static int may_create(struct inode *dir,
sbsec = dir->i_sb->s_security;
AVC_AUDIT_DATA_INIT(&ad, FS);
+ ad.u.fs.path.mnt = dir_path->mnt;
ad.u.fs.path.dentry = dentry;
rc = avc_has_perm(tsec->sid, dsec->sid, SECCLASS_DIR,
@@ -1485,11 +1487,12 @@ static int may_create_key(u32 ksid,
#define MAY_RMDIR 2
/* Check whether a task can link, unlink, or rmdir a file/directory. */
-static int may_link(struct inode *dir,
+static int may_link(struct path *dir_path,
struct dentry *dentry,
int kind)
{
+ struct inode *dir = dir_path->dentry->d_inode;
struct task_security_struct *tsec;
struct inode_security_struct *dsec, *isec;
struct avc_audit_data ad;
@@ -1501,6 +1504,7 @@ static int may_link(struct inode *dir,
isec = dentry->d_inode->i_security;
AVC_AUDIT_DATA_INIT(&ad, FS);
+ ad.u.fs.path.mnt = dir_path->mnt;
ad.u.fs.path.dentry = dentry;
av = DIR__SEARCH;
@@ -1529,11 +1533,13 @@ static int may_link(struct inode *dir,
return rc;
}
-static inline int may_rename(struct inode *old_dir,
+static inline int may_rename(struct path *old_dir_path,
struct dentry *old_dentry,
- struct inode *new_dir,
+ struct path *new_dir_path,
struct dentry *new_dentry)
{
+ struct inode *old_dir = old_dir_path->dentry->d_inode;
+ struct inode *new_dir = new_dir_path->dentry->d_inode;
struct task_security_struct *tsec;
struct inode_security_struct *old_dsec, *new_dsec, *old_isec, *new_isec;
struct avc_audit_data ad;
@@ -1549,6 +1555,7 @@ static inline int may_rename(struct inod
AVC_AUDIT_DATA_INIT(&ad, FS);
+ ad.u.fs.path.mnt = old_dir_path->mnt;
ad.u.fs.path.dentry = old_dentry;
rc = avc_has_perm(tsec->sid, old_dsec->sid, SECCLASS_DIR,
DIR__REMOVE_NAME | DIR__SEARCH, &ad);
@@ -1565,6 +1572,7 @@ static inline int may_rename(struct inod
return rc;
}
+ ad.u.fs.path.mnt = new_dir_path->mnt;
ad.u.fs.path.dentry = new_dentry;
av = DIR__ADD_NAME | DIR__SEARCH;
if (new_dentry->d_inode)
@@ -2485,7 +2493,7 @@ static int selinux_inode_init_security(s
static int selinux_inode_create(struct path *dir, struct dentry *dentry,
int mask)
{
- return may_create(dir->dentry->d_inode, dentry, SECCLASS_FILE);
+ return may_create(dir, dentry, SECCLASS_FILE);
}
static int selinux_inode_link(struct dentry *old_dentry, struct path *dir,
@@ -2496,7 +2504,7 @@ static int selinux_inode_link(struct den
rc = secondary_ops->inode_link(old_dentry, dir, new_dentry);
if (rc)
return rc;
- return may_link(dir->dentry->d_inode, old_dentry, MAY_LINK);
+ return may_link(dir, old_dentry, MAY_LINK);
}
static int selinux_inode_unlink(struct path *dir, struct dentry *dentry)
@@ -2506,24 +2514,24 @@ static int selinux_inode_unlink(struct p
rc = secondary_ops->inode_unlink(dir, dentry);
if (rc)
return rc;
- return may_link(dir->dentry->d_inode, dentry, MAY_UNLINK);
+ return may_link(dir, dentry, MAY_UNLINK);
}
static int selinux_inode_symlink(struct path *dir, struct dentry *dentry,
const char *name)
{
- return may_create(dir->dentry->d_inode, dentry, SECCLASS_LNK_FILE);
+ return may_create(dir, dentry, SECCLASS_LNK_FILE);
}
static int selinux_inode_mkdir(struct path *dir, struct dentry *dentry,
int mask)
{
- return may_create(dir->dentry->d_inode, dentry, SECCLASS_DIR);
+ return may_create(dir, dentry, SECCLASS_DIR);
}
static int selinux_inode_rmdir(struct path *dir, struct dentry *dentry)
{
- return may_link(dir->dentry->d_inode, dentry, MAY_RMDIR);
+ return may_link(dir, dentry, MAY_RMDIR);
}
static int selinux_inode_mknod(struct path *dir, struct dentry *dentry,
@@ -2535,15 +2543,15 @@ static int selinux_inode_mknod(struct pa
if (rc)
return rc;
- return may_create(dir->dentry->d_inode, dentry,
+ return may_create(dir, dentry,
inode_mode_to_security_class(mode));
}
static int selinux_inode_rename(struct path *old_dir, struct dentry *old_dentry,
struct path *new_dir, struct dentry *new_dentry)
{
- return may_rename(old_dir->dentry->d_inode, old_dentry,
- new_dir->dentry->d_inode, new_dentry);
+ return may_rename(old_dir, old_dentry,
+ new_dir, new_dentry);
}
static int selinux_inode_readlink(struct dentry *dentry)
@@ -2658,6 +2666,7 @@ static int selinux_inode_setxattr(struct
return -EPERM;
AVC_AUDIT_DATA_INIT(&ad, FS);
+ ad.u.fs.path.mnt = path->mnt;
ad.u.fs.path.dentry = dentry;
rc = avc_has_perm(tsec->sid, isec->sid, isec->sclass,