diff mbox

UBUNTU: SAUCE: apparmor: fix sleep from invalid context

Message ID 1454115195-5174-1-git-send-email-john.johansen@canonical.com
State New
Headers show

Commit Message

John Johansen Jan. 30, 2016, 12:53 a.m. UTC 
This is a patch to an apparmor feature that is not currently upstream

BugLink: http://bugs.launchpad.net/bugs/1539349

Commit 08518549722f0c992a9e4be71a0777f37147e9d2 made it so kern_path() via
getname_kernel() may do a GFP_KERNEL based allocation which is causing the
"sleep from invalid context" lockdep warning. Rework The apparmor mount
code to move kern_path() calls outside of the get_buffers()/put_buffers()
RCU read_lock block.

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/mount.c | 47 ++++++++++++++++++++++++-----------------------
 1 file changed, 24 insertions(+), 23 deletions(-)

Comments

Andy Whitcroft Jan. 30, 2016, 2:07 p.m. UTC | #1 
On Fri, Jan 29, 2016 at 04:53:15PM -0800, John Johansen wrote:
> This is a patch to an apparmor feature that is not currently upstream
> 
> BugLink: http://bugs.launchpad.net/bugs/1539349
> 
> Commit 08518549722f0c992a9e4be71a0777f37147e9d2 made it so kern_path() via
> getname_kernel() may do a GFP_KERNEL based allocation which is causing the
> "sleep from invalid context" lockdep warning. Rework The apparmor mount
> code to move kern_path() calls outside of the get_buffers()/put_buffers()
> RCU read_lock block.
> 
> Signed-off-by: John Johansen <john.johansen@canonical.com>

Looks reasonable.  Which series are affected by this?  I assume we have
these features in a number of branches these days ?

-apw
John Johansen Feb. 1, 2016, 7:08 p.m. UTC | #2 
On 01/30/2016 06:07 AM, Andy Whitcroft wrote:
> On Fri, Jan 29, 2016 at 04:53:15PM -0800, John Johansen wrote:
>> This is a patch to an apparmor feature that is not currently upstream
>>
>> BugLink: http://bugs.launchpad.net/bugs/1539349
>>
>> Commit 08518549722f0c992a9e4be71a0777f37147e9d2 made it so kern_path() via
>> getname_kernel() may do a GFP_KERNEL based allocation which is causing the
>> "sleep from invalid context" lockdep warning. Rework The apparmor mount
>> code to move kern_path() calls outside of the get_buffers()/put_buffers()
>> RCU read_lock block.
>>
>> Signed-off-by: John Johansen <john.johansen@canonical.com>
> 
> Looks reasonable.  Which series are affected by this?  I assume we have
> these features in a number of branches these days ?
> 

Sorry it applies to wily and xenial.
Brad Figg Feb. 1, 2016, 7:13 p.m. UTC | #3 
Applied to the master-next branch of Wily.
diff mbox

Patch

diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
index b2d3354..8c81157 100644
--- a/security/apparmor/mount.c
+++ b/security/apparmor/mount.c
@@ -381,13 +381,13 @@  int aa_bind_mount(struct aa_label *label, struct path *path,
 
 	flags &= MS_REC | MS_BIND;
 
-	get_buffers(buffer, old_buffer);
-	error = aa_path_name(path, path_flags(labels_profile(label), path), buffer, &name,
-			     &info, labels_profile(label)->disconnected);
+	error = kern_path(dev_name, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &old_path);
 	if (error)
 		goto error;
 
-	error = kern_path(dev_name, LOOKUP_FOLLOW|LOOKUP_AUTOMOUNT, &old_path);
+	get_buffers(buffer, old_buffer);
+	error = aa_path_name(path, path_flags(labels_profile(label), path), buffer, &name,
+			     &info, labels_profile(label)->disconnected);
 	if (error)
 		goto error;
 
@@ -463,6 +463,10 @@  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)
+		goto error;
+
 	get_buffers(buffer, old_buffer);
 	error = aa_path_name(path, path_flags(labels_profile(label), path),
 			     buffer, &name, &info,
@@ -470,10 +474,6 @@  int aa_move_mount(struct aa_label *label, struct path *path,
 	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,
@@ -508,22 +508,20 @@  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;
+	struct file_system_type *fstype;
+	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);
+		fstype = get_fs_type(type);
 		if (!fstype)
 			return -ENODEV;
-
 		binary = fstype->fs_flags & FS_BINARY_MOUNTDATA;
 		requires_dev = fstype->fs_flags & FS_REQUIRES_DEV;
 		put_filesystem(fstype);
 
 		if (requires_dev) {
-			struct path dev_path;
-
 			if (!dev_name || !*dev_name) {
 				error = -ENOENT;
 				goto out;
@@ -532,18 +530,21 @@  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,
-					     labels_profile(label)->disconnected);
-			path_put(&dev_path);
-			if (error)
-				goto error;
 		}
 	}
 
+	get_buffers(buffer, dev_buffer);
+	if (type) {
+		error = aa_path_name(&dev_path,
+				     path_flags(labels_profile(label),
+						&dev_path),
+				     dev_buffer, &dev_name, &info,
+				     labels_profile(label)->disconnected);
+		path_put(&dev_path);
+		if (error)
+			goto error;
+	}
+
 	error = aa_path_name(path, path_flags(labels_profile(label), path),
 			     buffer, &name, &info,
 			     labels_profile(label)->disconnected);