[apparmor] [PATCH] apparmor: fix leaked peer label in unix file checks

[Ryan Lee]

On Sat, May 2, 2026 at 11:55 PM Zygmunt Krynicki <me at zygoon.pl> wrote:
>
> aa_unix_file_perm() has an outer plabel variable that is released at
> function exit. The only assignment re-declares plabel in an inner scope,
> thus shadowing the variable from an outer scope. The reference returned by
> aa_get_label_rcu() is then assigned to the inner scope variable and leaks
> when that scope ends.
>
> Use the outer plabel so the existing exit-path aa_put_label() releases the
> peer label reference and the successful cache update sees the same label.
>
> Fixes: 88fec3526e84 ("apparmor: make sure unix socket labeling is correctly updated.")
>
> Signed-off-by: Zygmunt Krynicki <me at zygoon.pl>
> ---
>  security/apparmor/af_unix.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c
> index fdb4a9f212c3b..d7b1461a69635 100644
> --- a/security/apparmor/af_unix.c
> +++ b/security/apparmor/af_unix.c
> @@ -758,7 +758,6 @@ int aa_unix_file_perm(const struct cred *subj_cred, struct aa_label *label,
>                            unix_fs_perm(op, request, subj_cred, label,
>                                         is_unix_fs(peer_sk) ? &peer_path : NULL));
>         } else if (!is_sk_fs) {
> -               struct aa_label *plabel;
>                 struct aa_sk_ctx *pctx = aa_sock(peer_sk);

Also of interest is that the only assignments to a variable named
"plabel" occur inside this else-if block, which means that the
update_sk_ctx call in the cleanup also always did nothing because it
is always being called with a null plabel. Might there have been other
latent bugs being caused here besides of the resource leak?

>
>                 rcu_read_lock();
> @@ -796,4 +795,3 @@ int aa_unix_file_perm(const struct cred *subj_cred, struct aa_label *label,
>
>         return error;
>  }
> -
> --
> 2.53.0
>
>

It might make sense to try to move the cleanups around given that
plabel is only really used inside one of the if-else branches.
However, as this is a minimal patch fixing the issue:

Reviewed-by: Ryan Lee <ryan.lee at canonical.com>