On 26/09/12 18:34, Tim Gardner wrote:

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 <john.johansen@canonical.com>
Signed-off-by: Tim Gardner <tim.gardner@canonical.com>
---
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)
", __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);
}


Acked-by: Colin Ian King <colin.king@canonical.com>



--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team