Blob Blame History Raw
From b7a58e17ac5161a070b21e0b0f428474695a0591 Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Sat, 14 Sep 2019 03:34:06 -0700
Subject: [PATCH] apparmor: make it so work buffers can be allocated from
 atomic context

References: bnc#1158765
Patch-mainline: v5.5-rc1
Git-commit: 341c1fda5e17156619fb71acfc7082b2669b4b72

In some situations AppArmor needs to be able to use its work buffers
from atomic context. Add the ability to specify when in atomic context
and hold a set of work buffers in reserve for atomic context to
reduce the chance that a large work buffer allocation will need to
be done.

Fixes: df323337e507 ("apparmor: Use a memory pool instead per-CPU caches")
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Mel Gorman <mgorman@suse.de>
---
 security/apparmor/domain.c       |  2 +-
 security/apparmor/file.c         | 21 +++++++++--------
 security/apparmor/include/file.h |  2 +-
 security/apparmor/include/path.h |  3 ++-
 security/apparmor/lsm.c          | 50 ++++++++++++++++++++++++++++------------
 security/apparmor/mount.c        | 22 +++++++++---------
 6 files changed, 62 insertions(+), 38 deletions(-)

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index b1895fb81425..9be7ccb8379e 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -892,7 +892,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 		ctx->nnp = aa_get_label(label);
 
 	/* buffer freed below, name is pointer into buffer */
-	buffer = aa_get_buffer();
+	buffer = aa_get_buffer(false);
 	if (!buffer) {
 		error = -ENOMEM;
 		goto done;
diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index e2d087bc5d23..fe2ebe5e865e 100644
--- a/security/apparmor/file.c
+++ b/security/apparmor/file.c
@@ -332,7 +332,7 @@ int aa_path_perm(const char *op, struct aa_label *label,
 
 	flags |= PATH_DELEGATE_DELETED | (S_ISDIR(cond->mode) ? PATH_IS_DIR :
 								0);
-	buffer = aa_get_buffer();
+	buffer = aa_get_buffer(false);
 	if (!buffer)
 		return -ENOMEM;
 	error = fn_for_each_confined(label, profile,
@@ -477,8 +477,8 @@ int aa_path_link(struct aa_label *label, struct dentry *old_dentry,
 	int error;
 
 	/* buffer freed below, lname is pointer in buffer */
-	buffer = aa_get_buffer();
-	buffer2 = aa_get_buffer();
+	buffer = aa_get_buffer(false);
+	buffer2 = aa_get_buffer(false);
 	error = -ENOMEM;
 	if (!buffer || !buffer2)
 		goto out;
@@ -515,7 +515,7 @@ static void update_file_ctx(struct aa_file_ctx *fctx, struct aa_label *label,
 
 static int __file_path_perm(const char *op, struct aa_label *label,
 			    struct aa_label *flabel, struct file *file,
-			    u32 request, u32 denied)
+			    u32 request, u32 denied, bool in_atomic)
 {
 	struct aa_profile *profile;
 	struct aa_perms perms = {};
@@ -532,7 +532,7 @@ static int __file_path_perm(const char *op, struct aa_label *label,
 		return 0;
 
 	flags = PATH_DELEGATE_DELETED | (S_ISDIR(cond.mode) ? PATH_IS_DIR : 0);
-	buffer = aa_get_buffer();
+	buffer = aa_get_buffer(in_atomic);
 	if (!buffer)
 		return -ENOMEM;
 
@@ -600,11 +600,12 @@ static int __file_sock_perm(const char *op, struct aa_label *label,
  * @label: label being enforced   (NOT NULL)
  * @file: file to revalidate access permissions on  (NOT NULL)
  * @request: requested permissions
+ * @in_atomic: whether allocations need to be done in atomic context
  *
  * Returns: %0 if access allowed else error
  */
 int aa_file_perm(const char *op, struct aa_label *label, struct file *file,
-		 u32 request)
+		 u32 request, bool in_atomic)
 {
 	struct aa_file_ctx *fctx;
 	struct aa_label *flabel;
@@ -637,7 +638,7 @@ int aa_file_perm(const char *op, struct aa_label *label, struct file *file,
 
 	if (file->f_path.mnt && path_mediated_fs(file->f_path.dentry))
 		error = __file_path_perm(op, label, flabel, file, request,
-					 denied);
+					 denied, in_atomic);
 
 	else if (S_ISSOCK(file_inode(file)->i_mode))
 		error = __file_sock_perm(op, label, flabel, file, request,
@@ -665,7 +666,8 @@ static void revalidate_tty(struct aa_label *label)
 					     struct tty_file_private, list);
 		file = file_priv->file;
 
-		if (aa_file_perm(OP_INHERIT, label, file, MAY_READ | MAY_WRITE))
+		if (aa_file_perm(OP_INHERIT, label, file, MAY_READ | MAY_WRITE,
+				 IN_ATOMIC))
 			drop_tty = 1;
 	}
 	spin_unlock(&tty->files_lock);
@@ -679,7 +681,8 @@ static int match_file(const void *p, struct file *file, unsigned int fd)
 {
 	struct aa_label *label = (struct aa_label *)p;
 
-	if (aa_file_perm(OP_INHERIT, label, file, aa_map_file_to_perms(file)))
+	if (aa_file_perm(OP_INHERIT, label, file, aa_map_file_to_perms(file),
+			 IN_ATOMIC))
 		return fd + 1;
 	return 0;
 }
diff --git a/security/apparmor/include/file.h b/security/apparmor/include/file.h
index a852be89a7dc..aff26fc71407 100644
--- a/security/apparmor/include/file.h
+++ b/security/apparmor/include/file.h
@@ -197,7 +197,7 @@ int aa_path_link(struct aa_label *label, struct dentry *old_dentry,
 		 const struct path *new_dir, struct dentry *new_dentry);
 
 int aa_file_perm(const char *op, struct aa_label *label, struct file *file,
-		 u32 request);
+		 u32 request, bool in_atomic);
 
 void aa_inherit_files(const struct cred *cred, struct files_struct *files);
 
diff --git a/security/apparmor/include/path.h b/security/apparmor/include/path.h
index 8ef05f106ce9..44a7945fbe3c 100644
--- a/security/apparmor/include/path.h
+++ b/security/apparmor/include/path.h
@@ -25,7 +25,8 @@ int aa_path_name(const struct path *path, int flags, char *buffer,
 		 const char **name, const char **info,
 		 const char *disconnected);
 
-char *aa_get_buffer(void);
+#define IN_ATOMIC true
+char *aa_get_buffer(bool in_atomic);
 void aa_put_buffer(char *buf);
 
 #endif /* __AA_PATH_H */
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 87d8feae7edc..00624a065df5 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -48,6 +48,10 @@ union aa_buffer {
 	char buffer[1];
 };
 
+#define RESERVE_COUNT 2
+static int reserve_count = RESERVE_COUNT;
+static int buffer_count;
+
 static LIST_HEAD(aa_global_buffers);
 static DEFINE_SPINLOCK(aa_buffers_lock);
 
@@ -447,7 +451,8 @@ static void apparmor_file_free_security(struct file *file)
 		aa_put_label(rcu_access_pointer(ctx->label));
 }
 
-static int common_file_perm(const char *op, struct file *file, u32 mask)
+static int common_file_perm(const char *op, struct file *file, u32 mask,
+			    bool in_atomic)
 {
 	struct aa_label *label;
 	int error = 0;
@@ -457,7 +462,7 @@ static int common_file_perm(const char *op, struct file *file, u32 mask)
 		return -EACCES;
 
 	label = __begin_current_label_crit_section();
-	error = aa_file_perm(op, label, file, mask);
+	error = aa_file_perm(op, label, file, mask, in_atomic);
 	__end_current_label_crit_section(label);
 
 	return error;
@@ -465,12 +470,13 @@ static int common_file_perm(const char *op, struct file *file, u32 mask)
 
 static int apparmor_file_receive(struct file *file)
 {
-	return common_file_perm(OP_FRECEIVE, file, aa_map_file_to_perms(file));
+	return common_file_perm(OP_FRECEIVE, file, aa_map_file_to_perms(file),
+				false);
 }
 
 static int apparmor_file_permission(struct file *file, int mask)
 {
-	return common_file_perm(OP_FPERM, file, mask);
+	return common_file_perm(OP_FPERM, file, mask, false);
 }
 
 static int apparmor_file_lock(struct file *file, unsigned int cmd)
@@ -480,11 +486,11 @@ static int apparmor_file_lock(struct file *file, unsigned int cmd)
 	if (cmd == F_WRLCK)
 		mask |= MAY_WRITE;
 
-	return common_file_perm(OP_FLOCK, file, mask);
+	return common_file_perm(OP_FLOCK, file, mask, false);
 }
 
 static int common_mmap(const char *op, struct file *file, unsigned long prot,
-		       unsigned long flags)
+		       unsigned long flags, bool in_atomic)
 {
 	int mask = 0;
 
@@ -502,20 +508,21 @@ static int common_mmap(const char *op, struct file *file, unsigned long prot,
 	if (prot & PROT_EXEC)
 		mask |= AA_EXEC_MMAP;
 
-	return common_file_perm(op, file, mask);
+	return common_file_perm(op, file, mask, in_atomic);
 }
 
 static int apparmor_mmap_file(struct file *file, unsigned long reqprot,
 			      unsigned long prot, unsigned long flags)
 {
-	return common_mmap(OP_FMMAP, file, prot, flags);
+	return common_mmap(OP_FMMAP, file, prot, flags, GFP_ATOMIC);
 }
 
 static int apparmor_file_mprotect(struct vm_area_struct *vma,
 				  unsigned long reqprot, unsigned long prot)
 {
 	return common_mmap(OP_FMPROT, vma->vm_file, prot,
-			   !(vma->vm_flags & VM_SHARED) ? MAP_PRIVATE : 0);
+			   !(vma->vm_flags & VM_SHARED) ? MAP_PRIVATE : 0,
+			   false);
 }
 
 static int apparmor_sb_mount(const char *dev_name, const struct path *path,
@@ -1520,24 +1527,36 @@ static int param_set_mode(const char *val, const struct kernel_param *kp)
 	return 0;
 }
 
-char *aa_get_buffer(void)
+char *aa_get_buffer(bool in_atomic)
 {
 	union aa_buffer *aa_buf;
 	bool try_again = true;
+	gfp_t flags = (GFP_KERNEL | __GFP_RETRY_MAYFAIL | __GFP_NOWARN);
 
 retry:
 	spin_lock(&aa_buffers_lock);
-	if (!list_empty(&aa_global_buffers)) {
+	if (buffer_count > reserve_count ||
+	    (in_atomic && !list_empty(&aa_global_buffers))) {
 		aa_buf = list_first_entry(&aa_global_buffers, union aa_buffer,
 					  list);
 		list_del(&aa_buf->list);
+		buffer_count--;
 		spin_unlock(&aa_buffers_lock);
 		return &aa_buf->buffer[0];
 	}
+	if (in_atomic) {
+		/*
+		 * out of reserve buffers and in atomic context so increase
+		 * how many buffers to keep in reserve
+		 */
+		reserve_count++;
+		flags = GFP_ATOMIC;
+	}
 	spin_unlock(&aa_buffers_lock);
 
-	aa_buf = kmalloc(aa_g_path_max, GFP_KERNEL | __GFP_RETRY_MAYFAIL |
-			 __GFP_NOWARN);
+	if (!in_atomic)
+		might_sleep();
+	aa_buf = kmalloc(aa_g_path_max, flags);
 	if (!aa_buf) {
 		if (try_again) {
 			try_again = false;
@@ -1559,6 +1578,7 @@ void aa_put_buffer(char *buf)
 
 	spin_lock(&aa_buffers_lock);
 	list_add(&aa_buf->list, &aa_global_buffers);
+	buffer_count++;
 	spin_unlock(&aa_buffers_lock);
 }
 
@@ -1610,9 +1630,9 @@ static int __init alloc_buffers(void)
 	 * disabled early at boot if aa_g_path_max is extremly high.
 	 */
 	if (num_online_cpus() > 1)
-		num = 4;
+		num = 4 + RESERVE_COUNT;
 	else
-		num = 2;
+		num = 2 + RESERVE_COUNT;
 
 	for (i = 0; i < num; i++) {
 
diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
index f67ad5ac80b6..4ed6688f9d40 100644
--- a/security/apparmor/mount.c
+++ b/security/apparmor/mount.c
@@ -408,7 +408,7 @@ int aa_remount(struct aa_label *label, const struct path *path,
 
 	binary = path->dentry->d_sb->s_type->fs_flags & FS_BINARY_MOUNTDATA;
 
-	buffer = aa_get_buffer();
+	buffer = aa_get_buffer(false);
 	if (!buffer)
 		return -ENOMEM;
 	error = fn_for_each_confined(label, profile,
@@ -439,8 +439,8 @@ int aa_bind_mount(struct aa_label *label, const struct path *path,
 	if (error)
 		return error;
 
-	buffer = aa_get_buffer();
-	old_buffer = aa_get_buffer();
+	buffer = aa_get_buffer(false);
+	old_buffer = aa_get_buffer(false);
 	error = -ENOMEM;
 	if (!buffer || old_buffer)
 		goto out;
@@ -470,7 +470,7 @@ int aa_mount_change_type(struct aa_label *label, const struct path *path,
 	flags &= (MS_REC | MS_SILENT | MS_SHARED | MS_PRIVATE | MS_SLAVE |
 		  MS_UNBINDABLE);
 
-	buffer = aa_get_buffer();
+	buffer = aa_get_buffer(false);
 	if (!buffer)
 		return -ENOMEM;
 	error = fn_for_each_confined(label, profile,
@@ -499,8 +499,8 @@ int aa_move_mount(struct aa_label *label, const struct path *path,
 	if (error)
 		return error;
 
-	buffer = aa_get_buffer();
-	old_buffer = aa_get_buffer();
+	buffer = aa_get_buffer(false);
+	old_buffer = aa_get_buffer(false);
 	error = -ENOMEM;
 	if (!buffer || !old_buffer)
 		goto out;
@@ -550,13 +550,13 @@ int aa_new_mount(struct aa_label *label, const char *dev_name,
 		}
 	}
 
-	buffer = aa_get_buffer();
+	buffer = aa_get_buffer(false);
 	if (!buffer) {
 		error = -ENOMEM;
 		goto out;
 	}
 	if (dev_path) {
-		dev_buffer = aa_get_buffer();
+		dev_buffer = aa_get_buffer(false);
 		if (!dev_buffer) {
 			error = -ENOMEM;
 			goto out;
@@ -620,7 +620,7 @@ int aa_umount(struct aa_label *label, struct vfsmount *mnt, int flags)
 	AA_BUG(!label);
 	AA_BUG(!mnt);
 
-	buffer = aa_get_buffer();
+	buffer = aa_get_buffer(false);
 	if (!buffer)
 		return -ENOMEM;
 
@@ -699,8 +699,8 @@ int aa_pivotroot(struct aa_label *label, const struct path *old_path,
 	AA_BUG(!old_path);
 	AA_BUG(!new_path);
 
-	old_buffer = aa_get_buffer();
-	new_buffer = aa_get_buffer();
+	old_buffer = aa_get_buffer(false);
+	new_buffer = aa_get_buffer(false);
 	error = -ENOMEM;
 	if (!old_buffer || !new_buffer)
 		goto out;