* Re: [PATCH] apparmor: make unpacked rename and attach strings owned copies
2026-09-02 16:01 ` [PATCH] apparmor: make unpacked rename and attach strings owned copies Qihang
@ 2026-09-03 4:38 ` Greg KH
2026-11-03 17:26 ` Qihang
1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2026-09-03 4:38 UTC (permalink / raw)
To: Qihang; +Cc: john.johansen, georgia.garcia, security, stable
On Thu, Sep 03, 2026 at 12:01:38AM +0800, Qihang 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@vger.kernel.org
> Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
> ---
> security/apparmor/policy.c | 1 +
> security/apparmor/policy_unpack.c | 7 +++++--
> 2 files changed, 6 insertions(+), 2 deletions(-)
as this, and the other apparmor patch, are now public, please just do
this work on the public apparmor list, no need to get security@k.o
involved anymore.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] apparmor: make unpacked rename and attach strings owned copies
2026-09-02 16:01 ` [PATCH] apparmor: make unpacked rename and attach strings owned copies Qihang
2026-09-03 4:38 ` Greg KH
@ 2026-11-03 17:26 ` Qihang
1 sibling, 0 replies; 3+ messages in thread
From: Qihang @ 2026-11-03 17:26 UTC (permalink / raw)
To: john.johansen, georgia.garcia; +Cc: apparmor, linux-kernel
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@gmail.com> 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@vger.kernel.org
> Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
> ---
> 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)
>
^ permalink raw reply [flat|nested] 3+ messages in thread