From patchwork Wed Sep 26 17:34:01 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tim Gardner X-Patchwork-Id: 187154 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from chlorine.canonical.com (chlorine.canonical.com [91.189.94.204]) by ozlabs.org (Postfix) with ESMTP id 62BA02C007E for ; Thu, 27 Sep 2012 03:34:21 +1000 (EST) Received: from localhost ([127.0.0.1] helo=chlorine.canonical.com) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1TGvSP-00042S-29; Wed, 26 Sep 2012 17:31:33 +0000 Received: from mail.tpi.com ([70.99.223.143]) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1TGvSK-00042C-BL for kernel-team@lists.ubuntu.com; Wed, 26 Sep 2012 17:31:28 +0000 Received: from salmon.rtg.net (mail.tpi.com [70.99.223.143]) by mail.tpi.com (Postfix) with ESMTP id 795B832CCEA for ; Wed, 26 Sep 2012 10:33:26 -0700 (PDT) Received: by salmon.rtg.net (Postfix, from userid 1000) id CF97820BC3; Wed, 26 Sep 2012 11:34:01 -0600 (MDT) From: Tim Gardner To: kernel-team@lists.ubuntu.com Subject: [PATCH Lucid SRU] UBUNTU SAUCE: apparmor: fix IRQ stack overflow Date: Wed, 26 Sep 2012 11:34:01 -0600 Message-Id: <1348680841-64116-1-git-send-email-tim.gardner@canonical.com> X-Mailer: git-send-email 1.7.9.5 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.13 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: kernel-team-bounces@lists.ubuntu.com Errors-To: kernel-team-bounces@lists.ubuntu.com BugLink: http://bugs.launchpad.net/bugs/1056078 Profile replacement can cause a long chain of profiles to build up that get freed in a cascading chain of free_profile calls. Because free_profile is being called via aa_put_profile (and hence kref_put) each profile free is done via what amounts to recursion. That is free_profile indirectly calls free_profile on the next profile in the chain via aa_put_profile. Break this recursion by directly walking the chain, and as long as a profile is being freed because it has no more references continue on to the next profile. This results in at most 2 levels of free_profile being called. Signed-off-by: John Johansen Signed-off-by: Tim Gardner Acked-by: Colin Ian King --- security/apparmor/policy.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c index e1db319..415b8a9 100644 --- a/security/apparmor/policy.c +++ b/security/apparmor/policy.c @@ -659,6 +659,8 @@ fail: */ static void aa_free_profile(struct aa_profile *profile) { + struct aa_profile *p; + AA_DEBUG("%s(%p)\n", __func__, profile); if (!profile) @@ -685,8 +687,28 @@ static void aa_free_profile(struct aa_profile *profile) aa_free_sid(profile->sid); aa_put_dfa(profile->xmatch); - if (profile->replacedby) - aa_put_profile(profile->replacedby); + /* put the profile reference, but not via put_profile/kref_put + * replacedby can form a long chain that can result in cascading + * frees that blows the stack lp#1056078. The long chain creation + * should be addressed in profile replacement. + * This just addresses recursion of free_profile causing the + * stack to blow. + */ + for (p = profile->replacedby; p; ) { + if (atomic_dec_and_test(&p->base.count.refcount)) { + /* no more refs on p, grab its replacedby */ + struct aa_profile *next = p->replacedby; + /* break the chain */ + p->replacedby = NULL; + /* now free p, chain is broken */ + aa_put_profile(p); + + /* follow up with next profile in the chain */ + p = next; + } else + break; + } + kzfree(profile); }