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

[John Johansen]

sorry grabbed the questing patch, resend incoming

On 8/12/25 12:41, John Johansen wrote:
> 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);