Commit 2e680dd6 authored by John Johansen's avatar John Johansen Committed by James Morris

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: default avatarJohn Johansen <john.johansen@canonical.com>
Signed-off-by: default avatarJames Morris <james.l.morris@oracle.com>
parent 0e9e3e30
...@@ -724,6 +724,8 @@ struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat) ...@@ -724,6 +724,8 @@ struct aa_profile *aa_new_null_profile(struct aa_profile *parent, int hat)
*/ */
static void free_profile(struct aa_profile *profile) static void free_profile(struct aa_profile *profile)
{ {
struct aa_profile *p;
AA_DEBUG("%s(%p)\n", __func__, profile); AA_DEBUG("%s(%p)\n", __func__, profile);
if (!profile) if (!profile)
...@@ -751,7 +753,27 @@ static void free_profile(struct aa_profile *profile) ...@@ -751,7 +753,27 @@ static void free_profile(struct aa_profile *profile)
aa_put_dfa(profile->xmatch); aa_put_dfa(profile->xmatch);
aa_put_dfa(profile->policy.dfa); aa_put_dfa(profile->policy.dfa);
aa_put_profile(profile->replacedby); /* put the profile reference for replacedby, but not via
* put_profile(kref_put).
* replacedby can form a long chain that can result in cascading
* frees that blows the stack because kref_put makes a nested fn
* call (it looks like recursion, with free_profile calling
* free_profile) for each profile in the chain lp#1056078.
*/
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 */
free_profile(p);
/* follow up with next profile in the chain */
p = next;
} else
break;
}
kzfree(profile); kzfree(profile);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment