diff mbox

[trusty] UBUNTU: SAUCE: apparmor: fix sleep in critical section

Message ID 1cb4983e-4e3e-3e8c-4036-972335a61357@canonical.com
State New
Headers show

Commit Message

John Johansen Oct. 19, 2016, 3:40 p.m. UTC
kern_path() may do GFP_KERNEL based allocations and  path_put() calls
dput() which might sleep on some paths. When it does sleep from these
code paths, the per cpu work buffer may get reused overwriting the data
that was just placed in the buffer.

This causes the following mediation to fail as the work buffer no
longer has valid data for the current operation.

Move kern_path() and path_put() out of the critical section defined
by get_buffers()/put_buffers() and at the same time fix error cases
where the path was not being properly put.

BugLink: http://bugs.launchpad.net/bugs/1634753
Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/mount.c | 43 +++++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

Comments

Tim Gardner Oct. 19, 2016, 3:52 p.m. UTC | #1

Stefan Bader Oct. 24, 2016, 12:39 p.m. UTC | #2

Tim Gardner Oct. 26, 2016, 3 p.m. UTC | #3

diff mbox

Patch

diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
index 8e17122..076200e 100644
--- a/security/apparmor/mount.c
+++ b/security/apparmor/mount.c
@@ -380,20 +380,19 @@  int aa_bind_mount(struct aa_label *label, struct path *path,
 
 	flags &= MS_REC | MS_BIND;
 
+	error = kern_path(dev_name, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &old_path);
+	if (error)
+		return error;
+
 	get_buffers(buffer, old_buffer);
 	error = aa_path_name(path, path_flags(labels_profile(label), path), buffer, &name,
 			     &info);
 	if (error)
 		goto error;
 
-	error = kern_path(dev_name, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &old_path);
-	if (error)
-		goto error;
-
 	error = aa_path_name(&old_path, path_flags(labels_profile(label),
 						   &old_path),
 			     old_buffer, &old_name, &info);
-	path_put(&old_path);
 	if (error)
 		goto error;
 
@@ -403,6 +402,7 @@  int aa_bind_mount(struct aa_label *label, struct path *path,
 
 out:
 	put_buffers(buffer, old_buffer);
+	path_put(&old_path);
 
 	return error;
 
@@ -460,20 +460,19 @@  int aa_move_mount(struct aa_label *label, struct path *path,
 	if (!orig_name || !*orig_name)
 		return -EINVAL;
 
+	error = kern_path(orig_name, LOOKUP_FOLLOW, &old_path);
+	if (error)
+		return error;
+
 	get_buffers(buffer, old_buffer);
 	error = aa_path_name(path, path_flags(labels_profile(label), path),
 			     buffer, &name, &info);
 	if (error)
 		goto error;
 
-	error = kern_path(orig_name, LOOKUP_FOLLOW, &old_path);
-	if (error)
-		goto error;
-
 	error = aa_path_name(&old_path, path_flags(labels_profile(label),
 						   &old_path),
 			     old_buffer, &old_name, &info);
-	path_put(&old_path);
 	if (error)
 		goto error;
 
@@ -483,6 +482,7 @@  int aa_move_mount(struct aa_label *label, struct path *path,
 
 out:
 	put_buffers(buffer, old_buffer);
+	path_put(&old_path);
 
 	return error;
 
@@ -503,11 +503,11 @@  int aa_new_mount(struct aa_label *label, const char *orig_dev_name,
 	const char *name = NULL, *dev_name = NULL, *info = NULL;
 	bool binary = true;
 	int error;
+	int requires_dev = 0;
+	struct path dev_path;
 
 	dev_name = orig_dev_name;
-	get_buffers(buffer, dev_buffer);
 	if (type) {
-		int requires_dev;
 		struct file_system_type *fstype = get_fs_type(type);
 		if (!fstype)
 			return -ENODEV;
@@ -527,16 +527,17 @@  int aa_new_mount(struct aa_label *label, const char *orig_dev_name,
 			error = kern_path(dev_name, LOOKUP_FOLLOW, &dev_path);
 			if (error)
 				goto error;
-
-			error = aa_path_name(&dev_path,
-					     path_flags(labels_profile(label),
-							&dev_path),
-					     dev_buffer, &dev_name, &info);
-			path_put(&dev_path);
-			if (error)
-				goto error;
 		}
 	}
+	get_buffers(buffer, dev_buffer);
+	if (requires_dev) {
+		error = aa_path_name(&dev_path,
+				     path_flags(labels_profile(label),
+						&dev_path),
+				     dev_buffer, &dev_name, &info);
+		if (error)
+			goto error;
+	}
 
 	error = aa_path_name(path, path_flags(labels_profile(label), path),
 			     buffer, &name, &info);
@@ -549,6 +550,8 @@  int aa_new_mount(struct aa_label *label, const char *orig_dev_name,
 
 cleanup:
 	put_buffers(buffer, dev_buffer);
+	if (requires_dev)
+		path_put(&dev_path);
 
 out:
 	return error;