[apparmor] AppArmor Kernel-patch 3.6 in kernel 3.14 // for (p = profile->replacedby; p; )

[John Johansen]

On 11/23/2015 07:44 PM, Jae Suk Park wrote:
> I wonder if this patch should be applied or not,to 3.14 kernel (14.04LTS, Trusty)
> /kernel-patches/3.6/0006-apparmor-fix-IRQ-stack-overflow-during-free_profile.patch

No. The version of apparmor in 14.04 LTS (Trusty), is from the development branch
of apparmor 3. Which has not made it into the upstream kernel.

The development branch changed the way that replacements were handled.  In the
upstream 3.6 kernel a profile maintains a pointer to the profile that replaced it
creating a chain (as mentioned in the patch).

In the apparmor 3 development version, the chain was replacedby by a proxy struct
(struct replacedby) that the profiles point to, and that proxy then points to the
current version of the profile. This removed the problem of long replacement chains
that could problems referred to in the bug.

Is there are particular problem you are seeing that you are trying to solve?


> =================================
> From 663d5bbe6197bf990721c37ec877ea8ba5840202 Mon Sep 17 00:00:00 2001
> From: John Johansen <john.johansen at canonical.com <mailto:john.johansen at canonical.com>>
> Date: Wed, 24 Oct 2012 06:27:32 -0700
> Subject: [PATCH 6/6] apparmor: fix IRQ stack overflow during free_profile
> 
> BugLink: http://bugs.launchpad.net/bugs/1056078
> =================================
> It patches to security/apparmor/policy.c free_profile()
> and pointer mismatch in for loop statement in 3.14kernel (Trusty)
>    // for (p = profile->replacedby; p; ) 
> p points to aa_profile,but profile->replacedby points to different structure which has "kref count" element.
> ===============================
> security/apparmor/include/policy.h
> struct aa_replacedby {
>     struct kref count;
>     struct aa_profile __rcu *profile;
> };
> 
> struct aa_profile {
>     struct aa_policy base;
>     struct kref count;
>     struct rcu_head rcu;
>     struct aa_profile __rcu *parent;
> 
>     struct aa_namespace *ns;
>     struct aa_replacedby *replacedby;
>     const char *rename;
> 
> 
> security/apparmor/policy.c
> void aa_free_profile(struct aa_profile *profile)
> {
> struct aa_profile *p;
> 
> 
> /* 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;
> 
> 
>