apparmor: Fix regression in mount mediation
authorJohn Johansen <john.johansen@canonical.com>
Sun, 10 Sep 2023 10:35:22 +0000 (03:35 -0700)
committerJohn Johansen <john.johansen@canonical.com>
Wed, 18 Oct 2023 23:01:32 +0000 (16:01 -0700)
commit 2db154b3ea8e ("vfs: syscall: Add move_mount(2) to move mounts around")

introduced a new move_mount(2) system call and a corresponding new LSM
security_move_mount hook but did not implement this hook for any
existing LSM. This creates a regression for AppArmor mediation of
mount. This patch provides a base mapping of the move_mount syscall to
the existing mount mediation. In the future we may introduce
additional mediations around the new mount calls.

Fixes: 2db154b3ea8e ("vfs: syscall: Add move_mount(2) to move mounts around")
CC: stable@vger.kernel.org
Reported-by: Andreas Steinmetz <anstein99@googlemail.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
security/apparmor/include/mount.h
security/apparmor/lsm.c
security/apparmor/mount.c

index 10c76f9..46834f8 100644 (file)
@@ -38,9 +38,12 @@ int aa_mount_change_type(const struct cred *subj_cred,
                         struct aa_label *label, const struct path *path,
                         unsigned long flags);
 
+int aa_move_mount_old(const struct cred *subj_cred,
+                     struct aa_label *label, const struct path *path,
+                     const char *old_name);
 int aa_move_mount(const struct cred *subj_cred,
-                 struct aa_label *label, const struct path *path,
-                 const char *old_name);
+                 struct aa_label *label, const struct path *from_path,
+                 const struct path *to_path);
 
 int aa_new_mount(const struct cred *subj_cred,
                 struct aa_label *label, const char *dev_name,
index ce4f3e7..b047d1d 100644 (file)
@@ -722,8 +722,8 @@ static int apparmor_sb_mount(const char *dev_name, const struct path *path,
                        error = aa_mount_change_type(current_cred(), label,
                                                     path, flags);
                else if (flags & MS_MOVE)
-                       error = aa_move_mount(current_cred(), label, path,
-                                             dev_name);
+                       error = aa_move_mount_old(current_cred(), label, path,
+                                                 dev_name);
                else
                        error = aa_new_mount(current_cred(), label, dev_name,
                                             path, type, flags, data);
@@ -733,6 +733,21 @@ static int apparmor_sb_mount(const char *dev_name, const struct path *path,
        return error;
 }
 
+static int apparmor_move_mount(const struct path *from_path,
+                              const struct path *to_path)
+{
+       struct aa_label *label;
+       int error = 0;
+
+       label = __begin_current_label_crit_section();
+       if (!unconfined(label))
+               error = aa_move_mount(current_cred(), label, from_path,
+                                     to_path);
+       __end_current_label_crit_section(label);
+
+       return error;
+}
+
 static int apparmor_sb_umount(struct vfsmount *mnt, int flags)
 {
        struct aa_label *label;
@@ -1376,6 +1391,7 @@ static struct security_hook_list apparmor_hooks[] __ro_after_init = {
        LSM_HOOK_INIT(capget, apparmor_capget),
        LSM_HOOK_INIT(capable, apparmor_capable),
 
+       LSM_HOOK_INIT(move_mount, apparmor_move_mount),
        LSM_HOOK_INIT(sb_mount, apparmor_sb_mount),
        LSM_HOOK_INIT(sb_umount, apparmor_sb_umount),
        LSM_HOOK_INIT(sb_pivotroot, apparmor_sb_pivotroot),
index 3455dd4..fb30204 100644 (file)
@@ -483,36 +483,46 @@ int aa_mount_change_type(const struct cred *subj_cred,
 }
 
 int aa_move_mount(const struct cred *subj_cred,
-                 struct aa_label *label, const struct path *path,
-                 const char *orig_name)
+                 struct aa_label *label, const struct path *from_path,
+                 const struct path *to_path)
 {
        struct aa_profile *profile;
-       char *buffer = NULL, *old_buffer = NULL;
-       struct path old_path;
+       char *to_buffer = NULL, *from_buffer = NULL;
        int error;
 
        AA_BUG(!label);
-       AA_BUG(!path);
+       AA_BUG(!from_path);
+       AA_BUG(!to_path);
+
+       to_buffer = aa_get_buffer(false);
+       from_buffer = aa_get_buffer(false);
+       error = -ENOMEM;
+       if (!to_buffer || !from_buffer)
+               goto out;
+       error = fn_for_each_confined(label, profile,
+                       match_mnt(subj_cred, profile, to_path, to_buffer,
+                                 from_path, from_buffer,
+                                 NULL, MS_MOVE, NULL, false));
+out:
+       aa_put_buffer(to_buffer);
+       aa_put_buffer(from_buffer);
+
+       return error;
+}
+
+int aa_move_mount_old(const struct cred *subj_cred, struct aa_label *label,
+                     const struct path *path, const char *orig_name)
+{
+       struct path old_path;
+       int error;
 
        if (!orig_name || !*orig_name)
                return -EINVAL;
-
        error = kern_path(orig_name, LOOKUP_FOLLOW, &old_path);
        if (error)
                return error;
 
-       buffer = aa_get_buffer(false);
-       old_buffer = aa_get_buffer(false);
-       error = -ENOMEM;
-       if (!buffer || !old_buffer)
-               goto out;
-       error = fn_for_each_confined(label, profile,
-                       match_mnt(subj_cred, profile, path, buffer, &old_path,
-                                 old_buffer,
-                                 NULL, MS_MOVE, NULL, false));
-out:
-       aa_put_buffer(buffer);
-       aa_put_buffer(old_buffer);
+       error = aa_move_mount(subj_cred, label, &old_path, path);
        path_put(&old_path);
 
        return error;