[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