Commit ab092646 authored by Massimiliano Pellizzer's avatar Massimiliano Pellizzer Committed by John Johansen
Browse files

apparmor: replace recursive profile removal with iterative approach 



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: c88d4c7b ("AppArmor: core policy routines")
Reported-by: default avatarQualys Security Advisory <qsa@qualys.com>
Tested-by: default avatarSalvatore Bonaccorso <carnil@debian.org>
Reviewed-by: default avatarGeorgia Garcia <georgia.garcia@canonical.com>
Reviewed-by: default avatarCengiz Can <cengiz.can@canonical.com>
Signed-off-by: default avatarMassimiliano Pellizzer <massimiliano.pellizzer@canonical.com>
Signed-off-by: default avatarJohn Johansen <john.johansen@canonical.com>
parent e38c55d9

security/apparmor/policy.c

+27 −3
Original line number Diff line number Diff line
@@ -191,19 +191,43 @@ static void __list_remove_profile(struct aa_profile *profile) 
}



/**

 * __remove_profile - remove old profile, and children

 * @profile: profile to be replaced  (NOT NULL)

 * __remove_profile - remove profile, and children

 * @profile: profile to be removed  (NOT NULL)

 *

 * Requires: namespace list lock be held, or list not be shared

 */

static void __remove_profile(struct aa_profile *profile)

{

	struct aa_profile *curr, *to_remove;



	AA_BUG(!profile);

	AA_BUG(!profile->ns);

	AA_BUG(!mutex_is_locked(&profile->ns->lock));



	/* release any children lists first */

	__aa_profile_list_release(&profile->base.profiles);

	if (!list_empty(&profile->base.profiles)) {

		curr = list_first_entry(&profile->base.profiles, struct aa_profile, base.list);



		while (curr != profile) {



			while (!list_empty(&curr->base.profiles))

				curr = list_first_entry(&curr->base.profiles,

							struct aa_profile, base.list);



			to_remove = curr;

			if (!list_is_last(&to_remove->base.list,

					  &aa_deref_parent(curr)->base.profiles))

				curr = list_next_entry(to_remove, base.list);

			else

				curr = aa_deref_parent(curr);



			/* released by free_profile */

			aa_label_remove(&to_remove->label);

			__aafs_profile_rmdir(to_remove);

			__list_remove_profile(to_remove);

		}

	}



	/* released by free_profile */

	aa_label_remove(&profile->label);

	__aafs_profile_rmdir(profile);