[apparmor] apparmor: Fix regression in mount mediation
John Johansen
john.johansen at canonical.com
Sun Sep 10 11:59:40 UTC 2023
this is a first pass, I am going to work at reducing the diff size
From 09f20f314dc252bec4d8b21d84d60c0847b98b2a Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen at canonical.com>
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")
Signed-off-by: John Johansen <john.johansen at canonical.com>
---
security/apparmor/include/mount.h | 6 ++--
security/apparmor/lsm.c | 17 ++++++++++-
security/apparmor/mount.c | 47 ++++++++++++++++++-------------
3 files changed, 48 insertions(+), 22 deletions(-)
diff --git a/security/apparmor/include/mount.h b/security/apparmor/include/mount.h
index a710683b2496..3e2f974841e4 100644
--- a/security/apparmor/include/mount.h
+++ b/security/apparmor/include/mount.h
@@ -35,8 +35,10 @@ int aa_bind_mount(struct aa_label *label, const struct path *path,
int aa_mount_change_type(struct aa_label *label, const struct path *path,
unsigned long flags);
-int aa_move_mount(struct aa_label *label, const struct path *path,
- const char *old_name);
+int aa_move_mount_old(struct aa_label *label, const struct path *path,
+ const char *old_name);
+int aa_move_mount(struct aa_label *label, const struct path *from_path,
+ const struct path *to_path);
int aa_new_mount(struct aa_label *label, const char *dev_name,
const struct path *path, const char *type, unsigned long flags,
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index c9463bd0307d..4fa4d60a59fd 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -592,7 +592,7 @@ static int apparmor_sb_mount(const char *dev_name, const struct path *path,
MS_UNBINDABLE))
error = aa_mount_change_type(label, path, flags);
else if (flags & MS_MOVE)
- error = aa_move_mount(label, path, dev_name);
+ error = aa_move_mount_old(label, path, dev_name);
else
error = aa_new_mount(label, dev_name, path, type,
flags, data);
@@ -602,6 +602,20 @@ 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(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;
@@ -1221,6 +1235,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 cdfa430ae216..e992473b8d4a 100644
--- a/security/apparmor/mount.c
+++ b/security/apparmor/mount.c
@@ -468,40 +468,49 @@ int aa_mount_change_type(struct aa_label *label, const struct path *path,
return error;
}
-int aa_move_mount(struct aa_label *label, const struct path *path,
- const char *orig_name)
+
+int aa_move_mount(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);
-
- if (!orig_name || !*orig_name)
- return -EINVAL;
+ AA_BUG(!from_path);
+ AA_BUG(!to_path);
- error = kern_path(orig_name, LOOKUP_FOLLOW, &old_path);
- if (error)
- return error;
-
- buffer = aa_get_buffer(false);
- old_buffer = aa_get_buffer(false);
+ to_buffer = aa_get_buffer(false);
+ from_buffer = aa_get_buffer(false);
error = -ENOMEM;
- if (!buffer || !old_buffer)
+ if (!to_buffer || !from_buffer)
goto out;
error = fn_for_each_confined(label, profile,
- match_mnt(profile, path, buffer, &old_path, old_buffer,
+ match_mnt(profile, to_path, to_buffer,
+ from_path, from_buffer,
NULL, MS_MOVE, NULL, false));
out:
- aa_put_buffer(buffer);
- aa_put_buffer(old_buffer);
- path_put(&old_path);
+ aa_put_buffer(to_buffer);
+ aa_put_buffer(from_buffer);
return error;
}
+int aa_move_mount_old(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;
+
+ return aa_move_mount(label, &old_path, path);
+}
+
int aa_new_mount(struct aa_label *label, const char *dev_name,
const struct path *path, const char *type, unsigned long flags,
void *data)
--
2.34.1
More information about the AppArmor
mailing list