[SRU][P][PATCH 1/1] UBUNTU: SAUCE: apparmor5.0.0 [94/93]: apparmor: prevent profile->disconnected double free in aa_free_profile

[John Johansen]

BugLink: https://bugs.launchpad.net/bugs/2120233

policy_unpack.c:unpack_profile looks for strings labeled "disconnected"
and "disconnected_ipc", with the pointer stored for disconnected_ipc
falling back onto the pointer stored for disconnected if disconnected_ipc
is not present. However, policy.c:aa_free_profile unconditionally freed
both pointers, resulting in a double free if the pointers are identical.
To fix this, first check that the pointers are distinct before freeing the
disconnected_ipc pointer separately.

Under the right circumstances, KASAN would print a slab-use-after-free
message from aa_free_profile.part.0:

Call Trace:
  <snip>
  ? aa_free_profile.part.0
  ? aa_free_profile.part.0
  ? aa_free_profile.part.0
  __kasan_check_byte
  ? rcu_do_batch
  kfree_sensitive
  aa_free_profile.part.0
  ? rcu_do_batch
  aa_free_profile
  label_free_switch
  label_free_rcu
  rcu_do_batch
  <snip>
Allocated by task 6130:
  <snip>
  aa_unpack_strdup
  unpack_profile
  aa_unpack
  aa_replace_profiles
  policy_update
  profile_replace
  <snip>
Freed by task 17:
  kfree
  kfree_sensitive
  aa_free_profile.part.0
  aa_free_profile
  label_free_switch
  label_free_rcu
  rcu_do_batch
  <snip>

The double free could also lead to a kernel panic due to invalid opcodes
inside kfree:

  <snip>
  kfree_sensitive
  aa_free_profile.part.0
  ? rcu_do_batch
  aa_free_profile
  label_free_switch
  label_free_rcu
  rcu_do_batch

Suggested-by: John Johansen <john.johansen at canonical.com>
Fixes: 79a9a8a4c936 ("UBUNTU: SAUCE: apparmor4.0.0 [58/53]: apparmor: add mediation of disconnected paths in mqueues")
Signed-off-by: Ryan Lee <ryan.lee at canonical.com>
Signed-off-by: John Johansen <john.johansen at canonical.com>
---
  security/apparmor/policy.c | 9 ++++++++-
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
index 0a00fa0b9a22..d4b991d20521 100644
--- a/security/apparmor/policy.c
+++ b/security/apparmor/policy.c
@@ -310,7 +310,14 @@ void aa_free_profile(struct aa_profile *profile)
  	aa_put_ns(profile->ns);
  	kfree_sensitive(profile->rename);
  	kfree_sensitive(profile->disconnected);
-	kfree_sensitive(profile->disconnected_ipc);
+	/*
+	 * If disconnected is specified while disconnected_ipc is not,
+	 * disconnected_ipc will be set to disconnected in unpack_profile().
+	 * Thus, we need to check that the pointers are distinct in order to
+	 * prevent a double free.
+	 */
+	if (profile->disconnected_ipc != profile->disconnected)
+		kfree_sensitive(profile->disconnected_ipc);
  
  	free_attachment(&profile->attach);
  	kfree_sensitive(profile->net_compat);
-- 
2.43.0