From ed2c7da3a40c58410508fe24e12d03e508d7ec01 Mon Sep 17 00:00:00 2001 From: John Johansen Date: Mon, 14 Oct 2013 11:46:27 -0700 Subject: [PATCH] apparmor: fix bad lock balance when introspecting policy BugLink: http://bugs.launchpad.net/bugs/1235977 The profile introspection seq file has a locking bug when policy is viewed from a virtual root (task in a policy namespace), introspection from the real root is not affected. The test for root while (parent) { is correct for the real root, but incorrect for tasks in a policy namespace. This allows the task to walk backup the policy tree past its virtual root causing it to be unlocked before the virtual root should be in the p_stop fn. This results in the following lockdep back trace: [ 78.479744] [ BUG: bad unlock balance detected! ] [ 78.479792] 3.11.0-11-generic #17 Not tainted [ 78.479838] ------------------------------------- [ 78.479885] grep/2223 is trying to release lock (&ns->lock) at: [ 78.479952] [] mutex_unlock+0xe/0x10 [ 78.480002] but there are no more locks to release! [ 78.480037] [ 78.480037] other info that might help us debug this: [ 78.480037] 1 lock held by grep/2223: [ 78.480037] #0: (&p->lock){+.+.+.}, at: [] seq_read+0x3d/0x3d0 [ 78.480037] [ 78.480037] stack backtrace: [ 78.480037] CPU: 0 PID: 2223 Comm: grep Not tainted 3.11.0-11-generic #17 [ 78.480037] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 [ 78.480037] ffffffff817bf3be ffff880007763d60 ffffffff817b97ef ffff8800189d2190 [ 78.480037] ffff880007763d88 ffffffff810e1c6e ffff88001f044730 ffff8800189d2190 [ 78.480037] ffffffff817bf3be ffff880007763e00 ffffffff810e5bd6 0000000724fe56b7 [ 78.480037] Call Trace: [ 78.480037] [] ? mutex_unlock+0xe/0x10 [ 78.480037] [] dump_stack+0x54/0x74 [ 78.480037] [] print_unlock_imbalance_bug+0xee/0x100 [ 78.480037] [] ? mutex_unlock+0xe/0x10 [ 78.480037] [] lock_release_non_nested+0x226/0x300 [ 78.480037] [] ? __mutex_unlock_slowpath+0xce/0x180 [ 78.480037] [] ? mutex_unlock+0xe/0x10 [ 78.480037] [] lock_release+0xac/0x310 [ 78.480037] [] __mutex_unlock_slowpath+0x83/0x180 [ 78.480037] [] mutex_unlock+0xe/0x10 [ 78.480037] [] p_stop+0x51/0x90 [ 78.480037] [] seq_read+0x288/0x3d0 [ 78.480037] [] vfs_read+0x9e/0x170 [ 78.480037] [] SyS_read+0x4c/0xa0 [ 78.480037] [] system_call_fastpath+0x1a/0x1f Signed-off-by: John Johansen Signed-off-by: James Morris --- security/apparmor/apparmorfs.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c index 95c2b2689a03..7db9954f1af2 100644 --- a/security/apparmor/apparmorfs.c +++ b/security/apparmor/apparmorfs.c @@ -580,15 +580,13 @@ static struct aa_namespace *__next_namespace(struct aa_namespace *root, /* check if the next ns is a sibling, parent, gp, .. */ parent = ns->parent; - while (parent) { + while (ns != root) { mutex_unlock(&ns->lock); next = list_entry_next(ns, base.list); if (!list_entry_is_head(next, &parent->sub_ns, base.list)) { mutex_lock(&next->lock); return next; } - if (parent == root) - return NULL; ns = parent; parent = parent->parent; }