Skip to content

Commit 7eade84

Browse files
m-pellizzergregkh
authored andcommitted
apparmor: replace recursive profile removal with iterative approach
commit ab09264 upstream. The profile removal code uses recursion when removing nested profiles, which can lead to kernel stack exhaustion and system crashes. Reproducer: $ pf='a'; for ((i=0; i<1024; i++)); do echo -e "profile $pf { \n }" | apparmor_parser -K -a; pf="$pf//x"; done $ echo -n a > /sys/kernel/security/apparmor/.remove Replace the recursive __aa_profile_list_release() approach with an iterative approach in __remove_profile(). The function repeatedly finds and removes leaf profiles until the entire subtree is removed, maintaining the same removal semantic without recursion. Fixes: c88d4c7 ("AppArmor: core policy routines") Reported-by: Qualys Security Advisory <qsa@qualys.com> Tested-by: Salvatore Bonaccorso <carnil@debian.org> Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com> Reviewed-by: Cengiz Can <cengiz.can@canonical.com> Signed-off-by: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com> Signed-off-by: John Johansen <john.johansen@canonical.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 4f0889f commit 7eade84

1 file changed

Lines changed: 27 additions & 3 deletions

File tree

security/apparmor/policy.c

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,19 +183,43 @@ static void __list_remove_profile(struct aa_profile *profile)
183183
}
184184

185185
/**
186-
* __remove_profile - remove old profile, and children
187-
* @profile: profile to be replaced (NOT NULL)
186+
* __remove_profile - remove profile, and children
187+
* @profile: profile to be removed (NOT NULL)
188188
*
189189
* Requires: namespace list lock be held, or list not be shared
190190
*/
191191
static void __remove_profile(struct aa_profile *profile)
192192
{
193+
struct aa_profile *curr, *to_remove;
194+
193195
AA_BUG(!profile);
194196
AA_BUG(!profile->ns);
195197
AA_BUG(!mutex_is_locked(&profile->ns->lock));
196198

197199
/* release any children lists first */
198-
__aa_profile_list_release(&profile->base.profiles);
200+
if (!list_empty(&profile->base.profiles)) {
201+
curr = list_first_entry(&profile->base.profiles, struct aa_profile, base.list);
202+
203+
while (curr != profile) {
204+
205+
while (!list_empty(&curr->base.profiles))
206+
curr = list_first_entry(&curr->base.profiles,
207+
struct aa_profile, base.list);
208+
209+
to_remove = curr;
210+
if (!list_is_last(&to_remove->base.list,
211+
&aa_deref_parent(curr)->base.profiles))
212+
curr = list_next_entry(to_remove, base.list);
213+
else
214+
curr = aa_deref_parent(curr);
215+
216+
/* released by free_profile */
217+
aa_label_remove(&to_remove->label);
218+
__aafs_profile_rmdir(to_remove);
219+
__list_remove_profile(to_remove);
220+
}
221+
}
222+
199223
/* released by free_profile */
200224
aa_label_remove(&profile->label);
201225
__aafs_profile_rmdir(profile);

0 commit comments

Comments
 (0)