Re: apparmor: two more regression fixes for 3.12

Re: apparmor: two more regression fixes for 3.12

[James Morris]

On Sat, 12 Oct 2013, John Johansen wrote:

> Hi James,
> 
>   Can you please pull and forward the following 2 fixes for regressions in
>   3.12 apparmor
> 
> thanks
> john
> 
> ---
> 
> The following changes since commit 94ecad0c9ca2c9345013d2417081cea7cf842c16:
> 
>   apparmor: fix suspicious RCU usage warning in policy.c/policy.h (2013-09-29 08:28:11 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor security-next
> 
> for you to fetch changes up to dcb08d624b002693bed8ca6bf0456e09b4a90028:
> 
>   apparmor: fix bad lock balance when introspecting policy (2013-10-11 18:43:16 -0700)
> 
> ----------------------------------------------------------------
> John Johansen (2):
>       apparmor: fix memleak of the profile hash
>       apparmor: fix bad lock balance when introspecting policy
> 

Can you email me the patches?  I need to apply them to current Linus.



-- 
James Morris
<jmorris@namei.org>

Re: apparmor: two more regression fixes for 3.12

[John Johansen]

On 10/14/2013 08:20 AM, James Morris wrote:
> On Sat, 12 Oct 2013, John Johansen wrote:
> 
>> Hi James,
>>
>>   Can you please pull and forward the following 2 fixes for regressions in
>>   3.12 apparmor
>>

sure replied below or you can pull them from the branch below which I just rebased

The following changes since commit d6099aeb4a9aad5e7ab1c72eb119ebd52dee0d52:

  Merge branch 'fixes' of git://git.linaro.org/people/rmk/linux-arm (2013-10-14 10:02:23 -0700)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor for-security

for you to fetch changes up to 78aaf3232b29abbc83521814b946158b5c6293fc:

  apparmor: fix bad lock balance when introspecting policy (2013-10-14 11:31:34 -0700)

----------------------------------------------------------------
John Johansen (2):
      apparmor: fix memleak of the profile hash
      apparmor: fix bad lock balance when introspecting policy

 security/apparmor/apparmorfs.c | 4 +---
 security/apparmor/policy.c     | 1 +
 2 files changed, 2 insertions(+), 3 deletions(-)

[PATCH 1/2] apparmor: fix memleak of the profile hash

[John Johansen]

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

This fixes the following kmemleak trace:
unreferenced object 0xffff8801e8c35680 (size 32):
  comm "apparmor_parser", pid 691, jiffies 4294895667 (age 13230.876s)
  hex dump (first 32 bytes):
    e0 d3 4e b5 ac 6d f4 ed 3f cb ee 48 1c fd 40 cf  ..N..m..?..H..@.
    5b cc e9 93 00 00 00 00 00 00 00 00 00 00 00 00  [...............
  backtrace:
    [<ffffffff817a97ee>] kmemleak_alloc+0x4e/0xb0
    [<ffffffff811ca9f3>] __kmalloc+0x103/0x290
    [<ffffffff8138acbc>] aa_calc_profile_hash+0x6c/0x150
    [<ffffffff8138074d>] aa_unpack+0x39d/0xd50
    [<ffffffff8137eced>] aa_replace_profiles+0x3d/0xd80
    [<ffffffff81376937>] profile_replace+0x37/0x50
    [<ffffffff811e9f2d>] vfs_write+0xbd/0x1e0
    [<ffffffff811ea96c>] SyS_write+0x4c/0xa0
    [<ffffffff817ccb1d>] system_call_fastpath+0x1a/0x1f
    [<ffffffffffffffff>] 0xffffffffffffffff

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/policy.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 345bec0..705c287 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -610,6 +610,7 @@ void aa_free_profile(struct aa_profile *profile)
 	aa_put_dfa(profile->policy.dfa);
 	aa_put_replacedby(profile->replacedby);
 
+	kzfree(profile->hash);
 	kzfree(profile);
 }
 
-- 
1.8.3.2

[PATCH 2/2] apparmor: fix bad lock balance when introspecting policy

[John Johansen]

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] [<ffffffff817bf3be>] 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: [<ffffffff812111bd>] 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]  [<ffffffff817bf3be>] ? mutex_unlock+0xe/0x10
[   78.480037]  [<ffffffff817b97ef>] dump_stack+0x54/0x74
[   78.480037]  [<ffffffff810e1c6e>] print_unlock_imbalance_bug+0xee/0x100
[   78.480037]  [<ffffffff817bf3be>] ? mutex_unlock+0xe/0x10
[   78.480037]  [<ffffffff810e5bd6>] lock_release_non_nested+0x226/0x300
[   78.480037]  [<ffffffff817bf2fe>] ? __mutex_unlock_slowpath+0xce/0x180
[   78.480037]  [<ffffffff817bf3be>] ? mutex_unlock+0xe/0x10
[   78.480037]  [<ffffffff810e5d5c>] lock_release+0xac/0x310
[   78.480037]  [<ffffffff817bf2b3>] __mutex_unlock_slowpath+0x83/0x180
[   78.480037]  [<ffffffff817bf3be>] mutex_unlock+0xe/0x10
[   78.480037]  [<ffffffff81376c91>] p_stop+0x51/0x90
[   78.480037]  [<ffffffff81211408>] seq_read+0x288/0x3d0
[   78.480037]  [<ffffffff811e9d9e>] vfs_read+0x9e/0x170
[   78.480037]  [<ffffffff811ea8cc>] SyS_read+0x4c/0xa0
[   78.480037]  [<ffffffff817ccc9d>] system_call_fastpath+0x1a/0x1f

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 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 95c2b26..7db9954 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;
 	}
-- 
1.8.3.2