>From c2a1019449167109a021a0c8c23ea510cd69d334 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
Date: Sun, 18 May 2014 21:28:58 +0900
Subject: [PATCH] LSM: Pass the rename flags to each LSM module.
Commit da1ce067 "vfs: add cross-rename" changed security_inode_rename()
and security_path_rename() to call LSM module with reversed arguments
if the rename flags contains RENAME_EXCHANGE.
But that commit introduced unnecessary overhead for SMACK because
smack_inode_rename() does not need to be called with reversed arguments
when the rename flags contains RENAME_EXCHANGE. Also, that commit introduced
unnecessary overhead and unnecessary race window for TOMOYO and AppArmor
because they will have to calculate pathnames twice which are expected to
be identical between the call with non-reversed arguments and the call with
reversed arguments.
This patch fixes only unnecessary overhead for SMACK, by passing the rename
flags to each LSM module. Future patches will fix unnecessary overhead and
unnecessary race window for TOMOYO and AppArmor.
Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
Acked-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx> [smack]
Acked-by: Paul Moore <paul@xxxxxxxxxxxxxx> [selinux]
Acked-by: John Johansen <john.johansen@xxxxxxxxxxxxx> [apparmor]
---
include/linux/security.h | 8 ++++++--
security/apparmor/lsm.c | 14 +++++++++++++-
security/capability.c | 6 ++++--
security/security.c | 20 ++------------------
security/selinux/hooks.c | 8 ++++++--
security/smack/smack_lsm.c | 4 +++-
security/tomoyo/tomoyo.c | 9 +++++++--
7 files changed, 41 insertions(+), 28 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 6478ce3..5158611 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -446,6 +446,7 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
* @old_dentry contains the dentry structure of the old link.
* @new_dir contains the inode structure for parent of the new link.
* @new_dentry contains the dentry structure of the new link.
+ * @flags contains rename flags.
* Return 0 if permission is granted.
* @path_rename:
* Check for permission to rename a file or directory.
@@ -453,6 +454,7 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
* @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.
+ * @flags contains rename flags.
* Return 0 if permission is granted.
* @path_chmod:
* Check for permission to change DAC's permission of a file or directory.
@@ -1492,7 +1494,8 @@ struct security_operations {
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);
+ struct path *new_dir, struct dentry *new_dentry,
+ unsigned int flags);
int (*path_chmod) (struct path *path, umode_t mode);
int (*path_chown) (struct path *path, kuid_t uid, kgid_t gid);
int (*path_chroot) (struct path *path);
@@ -1515,7 +1518,8 @@ struct security_operations {
int (*inode_mknod) (struct inode *dir, struct dentry *dentry,
umode_t mode, dev_t dev);
int (*inode_rename) (struct inode *old_dir, struct dentry *old_dentry,
- struct inode *new_dir, struct dentry *new_dentry);
+ struct inode *new_dir, struct dentry *new_dentry,
+ unsigned int flags);
int (*inode_readlink) (struct dentry *dentry);
int (*inode_follow_link) (struct dentry *dentry, struct nameidata *nd);
int (*inode_permission) (struct inode *inode, int mask);
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 9981000..8f53e24 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -315,7 +315,8 @@ static int apparmor_path_link(struct dentry *old_dentry, struct path *new_dir,
}
static int apparmor_path_rename(struct path *old_dir, struct dentry *old_dentry,
- struct path *new_dir, struct dentry *new_dentry)
+ struct path *new_dir, struct dentry *new_dentry,
+ unsigned int flags)
{
struct aa_profile *profile;
int error = 0;
@@ -331,6 +332,7 @@ static int apparmor_path_rename(struct path *old_dir, struct dentry *old_dentry,
old_dentry->d_inode->i_mode
};
+retry:
error = aa_path_perm(OP_RENAME_SRC, profile, &old_path, 0,
MAY_READ | AA_MAY_META_READ | MAY_WRITE |
AA_MAY_META_WRITE | AA_MAY_DELETE,
@@ -339,6 +341,16 @@ static int apparmor_path_rename(struct path *old_dir, struct dentry *old_dentry,
error = aa_path_perm(OP_RENAME_DEST, profile, &new_path,
0, MAY_WRITE | AA_MAY_META_WRITE |
AA_MAY_CREATE, &cond);
+ if (!error && (flags & RENAME_EXCHANGE)) {
+ old_path.mnt = new_dir->mnt;
+ old_path.dentry = new_dentry;
+ new_path.mnt = old_dir->mnt;
+ new_path.dentry = old_dentry;
+ cond.uid = new_dentry->d_inode->i_uid;
+ cond.mode = new_dentry->d_inode->i_mode;
+ flags &= ~RENAME_EXCHANGE;
+ goto retry;
+ }
}
return error;
diff --git a/security/capability.c b/security/capability.c
index ad0d4de..620ba79 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -176,7 +176,8 @@ static int cap_inode_mknod(struct inode *inode, struct dentry *dentry,
}
static int cap_inode_rename(struct inode *old_inode, struct dentry *old_dentry,
- struct inode *new_inode, struct dentry *new_dentry)
+ struct inode *new_inode, struct dentry *new_dentry,
+ unsigned int flags)
{
return 0;
}
@@ -280,7 +281,8 @@ static int cap_path_link(struct dentry *old_dentry, struct path *new_dir,
}
static int cap_path_rename(struct path *old_path, struct dentry *old_dentry,
- struct path *new_path, struct dentry *new_dentry)
+ struct path *new_path, struct dentry *new_dentry,
+ unsigned int flags)
{
return 0;
}
diff --git a/security/security.c b/security/security.c
index 8b774f3..fbc4968 100644
--- a/security/security.c
+++ b/security/security.c
@@ -439,16 +439,8 @@ int security_path_rename(struct path *old_dir, struct dentry *old_dentry,
if (unlikely(IS_PRIVATE(old_dentry->d_inode) ||
(new_dentry->d_inode && IS_PRIVATE(new_dentry->d_inode))))
return 0;
-
- if (flags & RENAME_EXCHANGE) {
- int err = security_ops->path_rename(new_dir, new_dentry,
- old_dir, old_dentry);
- if (err)
- return err;
- }
-
return security_ops->path_rename(old_dir, old_dentry, new_dir,
- new_dentry);
+ new_dentry, flags);
}
EXPORT_SYMBOL(security_path_rename);
@@ -539,16 +531,8 @@ int security_inode_rename(struct inode *old_dir, struct dentry *old_dentry,
if (unlikely(IS_PRIVATE(old_dentry->d_inode) ||
(new_dentry->d_inode && IS_PRIVATE(new_dentry->d_inode))))
return 0;
-
- if (flags & RENAME_EXCHANGE) {
- int err = security_ops->inode_rename(new_dir, new_dentry,
- old_dir, old_dentry);
- if (err)
- return err;
- }
-
return security_ops->inode_rename(old_dir, old_dentry,
- new_dir, new_dentry);
+ new_dir, new_dentry, flags);
}
int security_inode_readlink(struct dentry *dentry)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 2c7341d..64d8497 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2749,9 +2749,13 @@ static int selinux_inode_mknod(struct inode *dir, struct dentry *dentry, umode_t
}
static int selinux_inode_rename(struct inode *old_inode, struct dentry *old_dentry,
- struct inode *new_inode, struct dentry *new_dentry)
+ struct inode *new_inode, struct dentry *new_dentry,
+ unsigned int flags)
{
- return may_rename(old_inode, old_dentry, new_inode, new_dentry);
+ int err = may_rename(old_inode, old_dentry, new_inode, new_dentry);
+ if (!err && (flags & RENAME_EXCHANGE))
+ err = may_rename(new_inode, new_dentry, old_inode, old_dentry);
+ return err;
}
static int selinux_inode_readlink(struct dentry *dentry)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 14f52be..4c1933f 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -697,6 +697,7 @@ static int smack_inode_rmdir(struct inode *dir, struct dentry *dentry)
* @old_dentry: unused
* @new_inode: the new directory
* @new_dentry: unused
+ * @flags: unused
*
* Read and write access is required on both the old and
* new directories.
@@ -706,7 +707,8 @@ static int smack_inode_rmdir(struct inode *dir, struct dentry *dentry)
static int smack_inode_rename(struct inode *old_inode,
struct dentry *old_dentry,
struct inode *new_inode,
- struct dentry *new_dentry)
+ struct dentry *new_dentry,
+ unsigned int flags)
{
int rc;
char *isp;
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index f0b756e..2c03812 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -287,17 +287,22 @@ static int tomoyo_path_link(struct dentry *old_dentry, struct path *new_dir,
* @old_dentry: Pointer to "struct dentry".
* @new_parent: Pointer to "struct path".
* @new_dentry: Pointer to "struct dentry".
+ * @flags: Rename flags.
*
* Returns 0 on success, negative value otherwise.
*/
static int tomoyo_path_rename(struct path *old_parent,
struct dentry *old_dentry,
struct path *new_parent,
- struct dentry *new_dentry)
+ struct dentry *new_dentry,
+ unsigned int flags)
{
struct path path1 = { old_parent->mnt, old_dentry };
struct path path2 = { new_parent->mnt, new_dentry };
- return tomoyo_path2_perm(TOMOYO_TYPE_RENAME, &path1, &path2);
+ int err = tomoyo_path2_perm(TOMOYO_TYPE_RENAME, &path1, &path2);
+ if (!err && (flags & RENAME_EXCHANGE))
+ err = tomoyo_path2_perm(TOMOYO_TYPE_RENAME, &path2, &path1);
+ return err;
}
/**
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
