LSM: Remove double path_rename hook calls for RENAME_EXCHANGE

In order to be able to identify a file exchange with renameat2(2) and
RENAME_EXCHANGE, which will be useful for Landlock [1], propagate the
rename flags to LSMs.  This may also improve performance because of the
switch from two set of LSM hook calls to only one, and because LSMs
using this hook may optimize the double check (e.g. only one lock,
reduce the number of path walks).

AppArmor, Landlock and Tomoyo are updated to leverage this change.  This
should not change the current behavior (same check order), except
(different level of) speed boosts.

[1] https://lore.kernel.org/r/20220221212522.320243-1-mic@digikod.net

Cc: James Morris <jmorris@namei.org>
Cc: Kentaro Takeda <takedakn@nttdata.co.jp>
Cc: Serge E. Hallyn <serge@hallyn.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reviewed-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20220506161102.525323-7-mic@digikod.net

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index db924fe379c9ca1b39fc453b19a6c617ef0f8ef9..eafa1d2489fdac3d7e48b1e24f7795b89b859887 100644 (file)
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -100,7 +100,7 @@ LSM_HOOK(int, 0, path_link, struct dentry *old_dentry,
         const struct path *new_dir, struct dentry *new_dentry)
 LSM_HOOK(int, 0, path_rename, const struct path *old_dir,
         struct dentry *old_dentry, const struct path *new_dir,
-        struct dentry *new_dentry)
+        struct dentry *new_dentry, unsigned int flags)
 LSM_HOOK(int, 0, path_chmod, const struct path *path, umode_t mode)
 LSM_HOOK(int, 0, path_chown, const struct path *path, kuid_t uid, kgid_t gid)
 LSM_HOOK(int, 0, path_chroot, const struct path *path)

diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 419b5febc3ca5ebad865bafda56df43e0f6eebcc..9acf5e368d73a55d3412a33d14340fc22ab05873 100644 (file)
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -358,6 +358,7 @@
  *     @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 may contain rename options such as RENAME_EXCHANGE.
  *     Return 0 if permission is granted.
  * @path_chmod:
  *     Check for permission to change a mode of the file @path. The new

diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 4f0eecb67dde09094d79d4e1483bea8baae5417d..900bc540656a247d2c07fab5a67a3a90dd3de824 100644 (file)
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -354,13 +354,16 @@ static int apparmor_path_link(struct dentry *old_dentry, const struct path *new_
 }
 
 static int apparmor_path_rename(const struct path *old_dir, struct dentry *old_dentry,
-                               const struct path *new_dir, struct dentry *new_dentry)
+                               const struct path *new_dir, struct dentry *new_dentry,
+                               const unsigned int flags)
 {
        struct aa_label *label;
        int error = 0;
 
        if (!path_mediated_fs(old_dentry))
                return 0;
+       if ((flags & RENAME_EXCHANGE) && !path_mediated_fs(new_dentry))
+               return 0;
 
        label = begin_current_label_crit_section();
        if (!unconfined(label)) {
@@ -374,10 +377,27 @@ static int apparmor_path_rename(const struct path *old_dir, struct dentry *old_d
                        d_backing_inode(old_dentry)->i_mode
                };
 
-               error = aa_path_perm(OP_RENAME_SRC, label, &old_path, 0,
-                                    MAY_READ | AA_MAY_GETATTR | MAY_WRITE |
-                                    AA_MAY_SETATTR | AA_MAY_DELETE,
-                                    &cond);
+               if (flags & RENAME_EXCHANGE) {
+                       struct path_cond cond_exchange = {
+                               i_uid_into_mnt(mnt_userns, d_backing_inode(new_dentry)),
+                               d_backing_inode(new_dentry)->i_mode
+                       };
+
+                       error = aa_path_perm(OP_RENAME_SRC, label, &new_path, 0,
+                                            MAY_READ | AA_MAY_GETATTR | MAY_WRITE |
+                                            AA_MAY_SETATTR | AA_MAY_DELETE,
+                                            &cond_exchange);
+                       if (!error)
+                               error = aa_path_perm(OP_RENAME_DEST, label, &old_path,
+                                                    0, MAY_WRITE | AA_MAY_SETATTR |
+                                                    AA_MAY_CREATE, &cond_exchange);
+               }
+
+               if (!error)
+                       error = aa_path_perm(OP_RENAME_SRC, label, &old_path, 0,
+                                            MAY_READ | AA_MAY_GETATTR | MAY_WRITE |
+                                            AA_MAY_SETATTR | AA_MAY_DELETE,
+                                            &cond);
                if (!error)
                        error = aa_path_perm(OP_RENAME_DEST, label, &new_path,
                                             0, MAY_WRITE | AA_MAY_SETATTR |

diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 7b7860039a08b97110cbe643f7b0061606208b23..30b42cdee52ebe77a328efe11a6bb434deb272ad 100644 (file)
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -622,10 +622,12 @@ static int hook_path_link(struct dentry *const old_dentry,
 static int hook_path_rename(const struct path *const old_dir,
                            struct dentry *const old_dentry,
                            const struct path *const new_dir,
-                           struct dentry *const new_dentry)
+                           struct dentry *const new_dentry,
+                           const unsigned int flags)
 {
        const struct landlock_ruleset *const dom =
                landlock_get_current_domain();
+       u32 exchange_access = 0;
 
        if (!dom)
                return 0;
@@ -633,12 +635,19 @@ static int hook_path_rename(const struct path *const old_dir,
        if (old_dir->dentry != new_dir->dentry)
                /* Gracefully forbids reparenting. */
                return -EXDEV;
+       if (flags & RENAME_EXCHANGE) {
+               if (unlikely(d_is_negative(new_dentry)))
+                       return -ENOENT;
+               exchange_access =
+                       get_mode_access(d_backing_inode(new_dentry)->i_mode);
+       }
        if (unlikely(d_is_negative(old_dentry)))
                return -ENOENT;
        /* RENAME_EXCHANGE is handled because directories are the same. */
        return check_access_path(
                dom, old_dir,
                maybe_remove(old_dentry) | maybe_remove(new_dentry) |
+                       exchange_access |
                        get_mode_access(d_backing_inode(old_dentry)->i_mode));
 }
 

diff --git a/security/security.c b/security/security.c
index b7cf5cbfdc677a37017e1e493f76d5644365bbf0..c9bfc0b70b288fababbcfa959c262707e89bfbe5 100644 (file)
--- a/security/security.c
+++ b/security/security.c
@@ -1197,15 +1197,8 @@ int security_path_rename(const struct path *old_dir, struct dentry *old_dentry,
                     (d_is_positive(new_dentry) && IS_PRIVATE(d_backing_inode(new_dentry)))))
                return 0;
 
-       if (flags & RENAME_EXCHANGE) {
-               int err = call_int_hook(path_rename, 0, new_dir, new_dentry,
-                                       old_dir, old_dentry);
-               if (err)
-                       return err;
-       }
-
        return call_int_hook(path_rename, 0, old_dir, old_dentry, new_dir,
-                               new_dentry);
+                               new_dentry, flags);
 }
 EXPORT_SYMBOL(security_path_rename);
 

diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index b6a31901f289477793b29af2456cd3ee18b225e0..71e82d855ebfca43bc3d9f0bb7a702bbcd0ce023 100644 (file)
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -264,17 +264,26 @@ static int tomoyo_path_link(struct dentry *old_dentry, const struct path *new_di
  * @old_dentry: Pointer to "struct dentry".
  * @new_parent: Pointer to "struct path".
  * @new_dentry: Pointer to "struct dentry".
+ * @flags: Rename options.
  *
  * Returns 0 on success, negative value otherwise.
  */
 static int tomoyo_path_rename(const struct path *old_parent,
                              struct dentry *old_dentry,
                              const struct path *new_parent,
-                             struct dentry *new_dentry)
+                             struct dentry *new_dentry,
+                             const unsigned int flags)
 {
        struct path path1 = { .mnt = old_parent->mnt, .dentry = old_dentry };
        struct path path2 = { .mnt = new_parent->mnt, .dentry = new_dentry };
 
+       if (flags & RENAME_EXCHANGE) {
+               const int err = tomoyo_path2_perm(TOMOYO_TYPE_RENAME, &path2,
+                               &path1);
+
+               if (err)
+                       return err;
+       }
        return tomoyo_path2_perm(TOMOYO_TYPE_RENAME, &path1, &path2);
 }