Skip to content

Commit 999bd70

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 786e2c2 commit 999bd70

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
@@ -184,19 +184,43 @@ static void __list_remove_profile(struct aa_profile *profile)
184184
}
185185

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

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

0 commit comments

Comments
 (0)