aboutsummaryrefslogtreecommitdiffstats
path: root/security
diff options
context:
space:
mode:
authorJohn Johansen <john.johansen@canonical.com>2012-10-24 09:27:32 -0400
committerJames Morris <james.l.morris@oracle.com>2012-10-24 11:12:50 -0400
commit2e680dd61e80592385338bfbeb86833d1c60546c (patch)
treea62b80465dd15a7fddb34367ccb7c94e47951dc5 /security
parent0e9e3e306c7e472bdcffa34c4c4584301eda03b3 (diff)
apparmor: fix IRQ stack overflow during free_profile
BugLink: http://bugs.launchpad.net/bugs/1056078 Profile replacement can cause long chains of profiles to build up when the profile being replaced is pinned. When the pinned profile is finally freed, it puts the reference to its replacement, which may in turn nest another call to free_profile on the stack. Because this may happen for each profile in the replacedby chain this can result in a recusion that causes the stack to overflow. Break this nesting by directly walking the chain of replacedby profiles (ie. use iteration instead of recursion to free the list). This results in at most 2 levels of free_profile being called, while freeing a replacedby chain. Signed-off-by: John Johansen <john.johansen@canonical.com> Signed-off-by: James Morris <james.l.morris@oracle.com>
Diffstat (limited to 'security')
-rw-r--r--security/apparmor/policy.c24
1 files changed, 23 insertions, 1 deletions
diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index cf5fd220309b..813200384d97 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -724,6 +724,8 @@ fail:
724 */ 724 */
725static void free_profile(struct aa_profile *profile) 725static void free_profile(struct aa_profile *profile)
726{ 726{
727 struct aa_profile *p;
728
727 AA_DEBUG("%s(%p)\n", __func__, profile); 729 AA_DEBUG("%s(%p)\n", __func__, profile);
728 730
729 if (!profile) 731 if (!profile)
@@ -751,7 +753,27 @@ static void free_profile(struct aa_profile *profile)
751 aa_put_dfa(profile->xmatch); 753 aa_put_dfa(profile->xmatch);
752 aa_put_dfa(profile->policy.dfa); 754 aa_put_dfa(profile->policy.dfa);
753 755
754 aa_put_profile(profile->replacedby); 756 /* put the profile reference for replacedby, but not via
757 * put_profile(kref_put).
758 * replacedby can form a long chain that can result in cascading
759 * frees that blows the stack because kref_put makes a nested fn
760 * call (it looks like recursion, with free_profile calling
761 * free_profile) for each profile in the chain lp#1056078.
762 */
763 for (p = profile->replacedby; p; ) {
764 if (atomic_dec_and_test(&p->base.count.refcount)) {
765 /* no more refs on p, grab its replacedby */
766 struct aa_profile *next = p->replacedby;
767 /* break the chain */
768 p->replacedby = NULL;
769 /* now free p, chain is broken */
770 free_profile(p);
771
772 /* follow up with next profile in the chain */
773 p = next;
774 } else
775 break;
776 }
755 777
756 kzfree(profile); 778 kzfree(profile);
757} 779}