Re: [PATCH] apparmor: make unpacked rename and attach strings owned copies

[Qihang]

Hi John,

Just a quick ping on this patch.

Thanks, Qihang

On Thu, Sep 3, 2026 at 12:02 AM Qihang <q.h.hack.winter@xxxxxxxxx> wrote:
>
> unpack_profile() stores the optional rename and attach strings using
> aa_unpack_str(), which returns pointers into the policy blob.
> profile->rename is later freed with kfree_sensitive(), causing an
> invalid free of an interior pointer on the failed-unpack path.
> profile->attach.xmatch_str can instead become a dangling pointer when
> the original policy blob is freed after compressed binary policy export.
> Use aa_unpack_strdup() for both strings and free xmatch_str with the
> attachment.
>
> Fixes: 736ec752d95e9 ("AppArmor: policy routines for loading and unpacking policy")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Qihang <q.h.hack.winter@xxxxxxxxx>
> ---
>  security/apparmor/policy.c        | 1 +
>  security/apparmor/policy_unpack.c | 7 +++++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
> index 94b4a7e727cc1..e9993cd53c903 100644
> --- a/security/apparmor/policy.c
> +++ b/security/apparmor/policy.c
> @@ -281,6 +281,7 @@ static void free_attachment(struct aa_attachment *attach)
>         for (i = 0; i < attach->xattr_count; i++)
>                 kfree_sensitive(attach->xattrs[i]);
>         kfree_sensitive(attach->xattrs);
> +       kfree_sensitive(attach->xmatch_str);
>         aa_put_pdb(attach->xmatch);
>  }
>
> diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
> index d9dcff167c480..9b534d56e38a5 100644
> --- a/security/apparmor/policy_unpack.c
> +++ b/security/apparmor/policy_unpack.c
> @@ -1120,6 +1120,7 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
>         size_t ns_len;
>         struct rhashtable_params params = { 0 };
>         char *key = NULL, *disconnected = NULL;
> +       char *rename = NULL, *attach_str = NULL;
>         struct aa_data *data;
>         int error = -EPROTO;
>         kernel_cap_t tmpcap;
> @@ -1159,10 +1160,12 @@ static struct aa_profile *unpack_profile(struct aa_ext *e, char **ns_name)
>         rules = profile->label.rules[0];
>
>         /* profile renaming is optional */
> -       (void) aa_unpack_str(e, &profile->rename, "rename");
> +       (void) aa_unpack_strdup(e, &rename, "rename");
> +       profile->rename = rename;
>
>         /* attachment string is optional */
> -       (void) aa_unpack_str(e, &profile->attach.xmatch_str, "attach");
> +       (void) aa_unpack_strdup(e, &attach_str, "attach");
> +       profile->attach.xmatch_str = attach_str;
>
>         /* xmatch is optional and may be NULL */
>         error = unpack_pdb(e, &profile->attach.xmatch, false, false, &info);
> --
> 2.50.1 (Apple Git-155)
>