On Sat, May 2, 2026 at 11:55 PM Zygmunt Krynicki <[email protected]> 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 <[email protected]> > --- > 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 <[email protected]>
