All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Johansen <john.johansen@canonical.com>
To: David Rientjes <rientjes@google.com>
Cc: Miles Lane <miles.lane@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Christoph Lameter <cl@linux-foundation.org>,
	Pekka Enberg <penberg@kernel.org>, Matt Mackall <mpm@selenic.com>,
	stable@kernel.org,
	"jmorris@namei.org >> James Morris" <jmorris@namei.org>
Subject: Re: 3.0.0-rc2-git1 -- BUG: sleeping function called from invalid context at mm/slub.c:847
Date: Wed, 08 Jun 2011 15:07:47 -0700	[thread overview]
Message-ID: <4DEFF2B3.7080202@canonical.com> (raw)
In-Reply-To: <alpine.DEB.2.00.1106081305430.10320@chino.kir.corp.google.com>

On 06/08/2011 01:09 PM, David Rientjes wrote:
> On Wed, 8 Jun 2011, Miles Lane wrote:
> 
>> BUG: sleeping function called from invalid context at mm/slub.c:847
>>  in_atomic(): 1, irqs_disabled(): 0, pid: 1583, name: cupsd
>>  2 locks held by cupsd/1583:
>>   #0:  (tasklist_lock){.+.+.+}, at: [<ffffffff8104dafa>] do_prlimit+0x61/0x189
>>   #1:  (&(&p->alloc_lock)->rlock){+.+.+.}, at: [<ffffffff8104db2d>]
>> do_prlimit+0x94/0x189
>>  Pid: 1583, comm: cupsd Not tainted 3.0.0-rc2-git1 #7
>>  Call Trace:
>>   [<ffffffff8102ebf2>] __might_sleep+0x10d/0x112
>>   [<ffffffff810e6f46>] slab_pre_alloc_hook.isra.49+0x2d/0x33
>>   [<ffffffff810e7bc4>] kmem_cache_alloc+0x22/0x132
>>   [<ffffffff8105b6e6>] prepare_creds+0x35/0xe4
>>   [<ffffffff811c0675>] aa_replace_current_profile+0x35/0xb2
>>   [<ffffffff811c4d2d>] aa_current_profile+0x45/0x4c
>>   [<ffffffff811c4d4d>] apparmor_task_setrlimit+0x19/0x3a
>>   [<ffffffff811beaa5>] security_task_setrlimit+0x11/0x13
>>   [<ffffffff8104db6b>] do_prlimit+0xd2/0x189
>>   [<ffffffff8104dea9>] sys_setrlimit+0x3b/0x48
>>   [<ffffffff814062bb>] system_call_fastpath+0x16/0x1b
>>
> 
> Not sure why this ever actually worked with apparmor if prepare_creds() 
> does an unconditional GFP_KERNEL allocation since this codepath hasn't 
> changed in at least a year and we're holding a spinlock from setrlimit.  
> John?

Hrmm, I am assuming it was missed because the profile replacement that
causes the prepare_creds() to be called is a rare condition when apparmor
is in use.  It will only occur when policy has been replaced and only if
the task doing the task_setrlimit() hasn't already updated its cred else
where.  It is clearly wrong, patch included below.

---

>From 99710968923ddc1f89029d98b2eb7d45b11323dc Mon Sep 17 00:00:00 2001
From: John Johansen <john.johansen@canonical.com>
Date: Wed, 8 Jun 2011 15:03:58 -0700
Subject: [PATCH] AppArmor: Fix sleep in invalid context from task_setrlimit

Affected kernels 2.6.36 - 3.0

AppArmor may do a GFP_KERNEL memory allocation with task_lock(tsk->group_leader);
held when called from security_task_setrlimit.  This will only occur when the
task's current policy has been replaced, and the task's creds have not been
updated before entering the LSM security_task_setrlimit() hook.

As reported by Miles Lane

BUG: sleeping function called from invalid context at mm/slub.c:847
 in_atomic(): 1, irqs_disabled(): 0, pid: 1583, name: cupsd
 2 locks held by cupsd/1583:
  #0:  (tasklist_lock){.+.+.+}, at: [<ffffffff8104dafa>] do_prlimit+0x61/0x189
  #1:  (&(&p->alloc_lock)->rlock){+.+.+.}, at: [<ffffffff8104db2d>]
do_prlimit+0x94/0x189
 Pid: 1583, comm: cupsd Not tainted 3.0.0-rc2-git1 #7
 Call Trace:
  [<ffffffff8102ebf2>] __might_sleep+0x10d/0x112
  [<ffffffff810e6f46>] slab_pre_alloc_hook.isra.49+0x2d/0x33
  [<ffffffff810e7bc4>] kmem_cache_alloc+0x22/0x132
  [<ffffffff8105b6e6>] prepare_creds+0x35/0xe4
  [<ffffffff811c0675>] aa_replace_current_profile+0x35/0xb2
  [<ffffffff811c4d2d>] aa_current_profile+0x45/0x4c
  [<ffffffff811c4d4d>] apparmor_task_setrlimit+0x19/0x3a
  [<ffffffff811beaa5>] security_task_setrlimit+0x11/0x13
  [<ffffffff8104db6b>] do_prlimit+0xd2/0x189
  [<ffffffff8104dea9>] sys_setrlimit+0x3b/0x48
  [<ffffffff814062bb>] system_call_fastpath+0x16/0x1b

Signed-off-by: John Johansen <john.johansen@canonical.com>
---
 security/apparmor/lsm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index ec1bcec..3d2fd14 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -612,7 +612,7 @@ static int apparmor_setprocattr(struct task_struct *task, char *name,
 static int apparmor_task_setrlimit(struct task_struct *task,
                unsigned int resource, struct rlimit *new_rlim)
 {
-       struct aa_profile *profile = aa_current_profile();
+       struct aa_profile *profile = __aa_current_profile();
        int error = 0;
 
        if (!unconfined(profile))
-- 
1.7.4.1


  parent reply	other threads:[~2011-06-08 22:07 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-08 19:02 3.0.0-rc2-git1 -- BUG: sleeping function called from invalid context at mm/slub.c:847 Miles Lane
2011-06-08 20:09 ` David Rientjes
2011-06-08 20:17   ` Matt Mackall
2011-06-08 21:34     ` David Rientjes
2011-06-08 21:57       ` Kyle Moffett
2011-06-08 22:12       ` John Johansen
2011-06-08 22:07   ` John Johansen [this message]
2011-06-08 23:47     ` [stable] " Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4DEFF2B3.7080202@canonical.com \
    --to=john.johansen@canonical.com \
    --cc=cl@linux-foundation.org \
    --cc=jmorris@namei.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miles.lane@gmail.com \
    --cc=mpm@selenic.com \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=stable@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.