[PATCH] apparmor: Fix label reference leak in aa_unix_file_perm()

[PATCH] apparmor: Fix label reference leak in aa_unix_file_perm()

[Wentao Liang]

The inner block in aa_unix_file_perm() re-declares plabel, shadowing the
outer variable that is released at the out label. The reference taken by
aa_get_label_rcu() is thus lost when the block exits, and aa_put_label()
at out is a no-op on the still NULL outer plabel, leaking one label per
permission check on a non-filesystem unix socket.

Drop the shadowing declaration so the outer plabel is used and released.

Fixes: 88fec3526e84 ("apparmor: make sure unix socket labeling is correctly updated.")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 security/apparmor/af_unix.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c
index fdb4a9f212c3..ecb40cfe7e14 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);
 
 		rcu_read_lock();
-- 
2.34.1

Re: [PATCH] apparmor: Fix label reference leak in aa_unix_file_perm()

[Ryan Lee]

On Thu, Sep 17, 2026 at 11:59 AM Wentao Liang <vulab@iscas.ac.cn> wrote:
>
> The inner block in aa_unix_file_perm() re-declares plabel, shadowing the
> outer variable that is released at the out label. The reference taken by
> aa_get_label_rcu() is thus lost when the block exits, and aa_put_label()
> at out is a no-op on the still NULL outer plabel, leaking one label per
> permission check on a non-filesystem unix socket.
>
> Drop the shadowing declaration so the outer plabel is used and released.
>
> Fixes: 88fec3526e84 ("apparmor: make sure unix socket labeling is correctly updated.")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  security/apparmor/af_unix.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c
> index fdb4a9f212c3..ecb40cfe7e14 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);
>
>                 rcu_read_lock();
> --
> 2.34.1
>
>

NACK: this exact patch has been proposed before, and unfortunately
exposes an additional latent bug around plabel handling that results
in a use-after-free if the shadowed declaration is removed without
additional fixes.

I can try to dig out the relevant exchange from the AppArmor mailing
list later, if you'd be interested.

Ryan

Re: [PATCH] apparmor: Fix label reference leak in aa_unix_file_perm()

[John Johansen]

On 9/17/26 12:19, Ryan Lee wrote:
> On Thu, Sep 17, 2026 at 11:59 AM Wentao Liang <vulab@iscas.ac.cn> wrote:
>>
>> The inner block in aa_unix_file_perm() re-declares plabel, shadowing the
>> outer variable that is released at the out label. The reference taken by
>> aa_get_label_rcu() is thus lost when the block exits, and aa_put_label()
>> at out is a no-op on the still NULL outer plabel, leaking one label per
>> permission check on a non-filesystem unix socket.
>>
>> Drop the shadowing declaration so the outer plabel is used and released.
>>
>> Fixes: 88fec3526e84 ("apparmor: make sure unix socket labeling is correctly updated.")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
>> ---
>>   security/apparmor/af_unix.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c
>> index fdb4a9f212c3..ecb40cfe7e14 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);
>>
>>                  rcu_read_lock();
>> --
>> 2.34.1
>>
>>
> 
> NACK: this exact patch has been proposed before, and unfortunately
> exposes an additional latent bug around plabel handling that results
> in a use-after-free if the shadowed declaration is removed without
> additional fixes.
> 
> I can try to dig out the relevant exchange from the AppArmor mailing
> list later, if you'd be interested.
> 
The series Ryan is referring to is in Linus's tree, so you should see the
fix rolling out to stable kernels as well soon.

6d25e7b47616c apparmor: fix refcount leak when updating the sk_ctx
b1aea2c196077 apparmor: fix race in unix socket mediation when peer_path is used
4483efe4f2151 apparmor: fix shadowing of plabel that prevents cache from being updated

> Ryan

Re: [PATCH] apparmor: Fix label reference leak in aa_unix_file_perm()

[Greg KH]

On Thu, Sep 17, 2026 at 01:18:20PM -0700, John Johansen wrote:
> On 9/17/26 12:19, Ryan Lee wrote:
> > On Thu, Sep 17, 2026 at 11:59 AM Wentao Liang <vulab@iscas.ac.cn> wrote:
> > > 
> > > The inner block in aa_unix_file_perm() re-declares plabel, shadowing the
> > > outer variable that is released at the out label. The reference taken by
> > > aa_get_label_rcu() is thus lost when the block exits, and aa_put_label()
> > > at out is a no-op on the still NULL outer plabel, leaking one label per
> > > permission check on a non-filesystem unix socket.
> > > 
> > > Drop the shadowing declaration so the outer plabel is used and released.
> > > 
> > > Fixes: 88fec3526e84 ("apparmor: make sure unix socket labeling is correctly updated.")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> > > ---
> > >   security/apparmor/af_unix.c | 1 -
> > >   1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c
> > > index fdb4a9f212c3..ecb40cfe7e14 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);
> > > 
> > >                  rcu_read_lock();
> > > --
> > > 2.34.1
> > > 
> > > 
> > 
> > NACK: this exact patch has been proposed before, and unfortunately
> > exposes an additional latent bug around plabel handling that results
> > in a use-after-free if the shadowed declaration is removed without
> > additional fixes.
> > 
> > I can try to dig out the relevant exchange from the AppArmor mailing
> > list later, if you'd be interested.
> > 
> The series Ryan is referring to is in Linus's tree, so you should see the
> fix rolling out to stable kernels as well soon.
> 
> 6d25e7b47616c apparmor: fix refcount leak when updating the sk_ctx
> b1aea2c196077 apparmor: fix race in unix socket mediation when peer_path is used
> 4483efe4f2151 apparmor: fix shadowing of plabel that prevents cache from being updated

These are all in stable released kernels already for many weeks, so how
is any of this still being reported?

confused,

greg k-h

Re: [PATCH] apparmor: Fix label reference leak in aa_unix_file_perm()

[John Johansen]

On 9/18/26 01:24, Greg KH wrote:
> On Thu, Sep 17, 2026 at 01:18:20PM -0700, John Johansen wrote:
>> On 9/17/26 12:19, Ryan Lee wrote:
>>> On Thu, Sep 17, 2026 at 11:59 AM Wentao Liang <vulab@iscas.ac.cn> wrote:
>>>>
>>>> The inner block in aa_unix_file_perm() re-declares plabel, shadowing the
>>>> outer variable that is released at the out label. The reference taken by
>>>> aa_get_label_rcu() is thus lost when the block exits, and aa_put_label()
>>>> at out is a no-op on the still NULL outer plabel, leaking one label per
>>>> permission check on a non-filesystem unix socket.
>>>>
>>>> Drop the shadowing declaration so the outer plabel is used and released.
>>>>
>>>> Fixes: 88fec3526e84 ("apparmor: make sure unix socket labeling is correctly updated.")
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
>>>> ---
>>>>    security/apparmor/af_unix.c | 1 -
>>>>    1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c
>>>> index fdb4a9f212c3..ecb40cfe7e14 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);
>>>>
>>>>                   rcu_read_lock();
>>>> --
>>>> 2.34.1
>>>>
>>>>
>>>
>>> NACK: this exact patch has been proposed before, and unfortunately
>>> exposes an additional latent bug around plabel handling that results
>>> in a use-after-free if the shadowed declaration is removed without
>>> additional fixes.
>>>
>>> I can try to dig out the relevant exchange from the AppArmor mailing
>>> list later, if you'd be interested.
>>>
>> The series Ryan is referring to is in Linus's tree, so you should see the
>> fix rolling out to stable kernels as well soon.
>>
>> 6d25e7b47616c apparmor: fix refcount leak when updating the sk_ctx
>> b1aea2c196077 apparmor: fix race in unix socket mediation when peer_path is used
>> 4483efe4f2151 apparmor: fix shadowing of plabel that prevents cache from being updated
> 
> These are all in stable released kernels already for many weeks, so how
> is any of this still being reported?
> 
> confused,
> I don't know, my guess is either using a distro kernel, or not having updated
to a newer stable kernel. I should not have said "stable", I should have said in
updated kernels.