From 157a3537d6bc28ceb9a11fc8cb67f2152d860146 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Sun, 10 Sep 2023 03:35:22 -0700 Subject: [PATCH] apparmor: Fix regression in mount mediation 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 Signed-off-by: John Johansen --- security/apparmor/include/mount.h | 7 +++-- security/apparmor/lsm.c | 20 ++++++++++++-- security/apparmor/mount.c | 46 +++++++++++++++++++------------ 3 files changed, 51 insertions(+), 22 deletions(-) diff --git a/security/apparmor/include/mount.h b/security/apparmor/include/mount.h index 10c76f906a65..46834f828179 100644 --- a/security/apparmor/include/mount.h +++ b/security/apparmor/include/mount.h @@ -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, diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c index ce4f3e7a784d..b047d1d355a9 100644 --- a/security/apparmor/lsm.c +++ b/security/apparmor/lsm.c @@ -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), diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c index 3455dd4b1f99..fb30204c761a 100644 --- a/security/apparmor/mount.c +++ b/security/apparmor/mount.c @@ -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;