On August 24, 2026 5:58 pm, Maxime Bélair via AppArmor wrote:
> aa_unix_file_perm lazily refreshes the AppArmor context cached on a unix
> socket. Two of the helpers it uses assume ctx->peer has already been set:
> 
>       update_peer_ctx ->  l = aa_label_merge(old, label, GFP_ATOMIC);
>       update_sk_ctx   ->  } else if (aa_label_is_subset(plabel, old)) {
> 
> where @old is ctx->peer. Neither aa_label_merge nor aa_label_is_subset
> allows NULL. So both fault on the aa_label->size load.
> 
>   BUG: kernel NULL pointer dereference, address: 000000000000004c
>   RIP: 0010:__aa_label_next_not_in_set+0xb/0xd0
>   Call Trace:
>    aa_label_is_subset+0x3f/0x70
>    aa_unix_file_perm+0x5e8/0x9d0
>    aa_file_perm+0x45a/0x550
>    apparmor_file_permission+0x44/0xb0
>    security_file_permission+0x40/0x100
>    rw_verify_area+0x56/0x180
>    vfs_write+0x7c/0x480
>    ksys_write+0xbf/0xf0
> 
> ctx->peer is only recorded for stream connections and socket pairs.
> unix_dgram_connect sets unix_peer(sk) without going through that path,
> so a connected AF_UNIX datagram socket has unix_peer(sk) set while
> ctx->peer is still NULL, and the first write that needs revalidation
> reaches the helpers above.
> 
> Both derefs date back to the Fixes: commit, but the update_sk_ctx() one
> was dormant until commit 4483efe4f215 ("apparmor: fix shadowing of plabel
> that prevents cache from being updated") stopped @plabel being shadowed,
> which is why bisecting the oops lands there.
> 
> A NULL @old just means no peer label has been recorded yet, so install
> the label directly instead of merging or comparing against it.
> 
> Fixes: 88fec3526e84 ("apparmor: make sure unix socket labeling is correctly 
> updated.")
> Reported-by: Aurelien Jarno <[email protected]>
> Closes: https://bugs.debian.org/1145111
> Cc: [email protected]
> Signed-off-by: Maxime Bélair <[email protected]>

FWIW, this triggers when running rustc's test suite on a system with AA
enabled, and I can confirm that the patch fixes the issue/prevents an
oops.

Tested-by: Fabian Grünbichler <[email protected]>

> ---
>  security/apparmor/af_unix.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c
> index b908e744818c..d04d9cd268aa 100644
> --- a/security/apparmor/af_unix.c
> +++ b/security/apparmor/af_unix.c
> @@ -682,7 +682,7 @@ static void update_sk_ctx(struct sock *sk, struct 
> aa_label *label,
>               if (old == plabel) {
>                       rcu_assign_pointer(ctx->peer_lastupdate,
>                                          aa_get_label(plabel));
> -             } else if (aa_label_is_subset(plabel, old)) {
> +             } else if (!old || aa_label_is_subset(plabel, old)) {
>                       rcu_assign_pointer(ctx->peer_lastupdate,
>                                          aa_get_label(plabel));
>                       rcu_assign_pointer(ctx->peer, aa_get_label(plabel));
> @@ -700,13 +700,17 @@ static void update_peer_ctx(struct sock *sk, struct 
> aa_sk_ctx *ctx,
>       spin_lock(&unix_sk(sk)->lock);
>       old = rcu_dereference_protected(ctx->peer,
>                                       lockdep_is_held(&unix_sk(sk)->lock));
> -     l = aa_label_merge(old, label, GFP_ATOMIC);
> -     if (l) {
> -             if (l != old) {
> -                     rcu_assign_pointer(ctx->peer, l);
> -                     aa_put_label(old);
> -             } else
> -                     aa_put_label(l);
> +     if (!old)
> +             rcu_assign_pointer(ctx->peer, aa_get_label(label));
> +     else {
> +             l = aa_label_merge(old, label, GFP_ATOMIC);
> +             if (l) {
> +                     if (l != old) {
> +                             rcu_assign_pointer(ctx->peer, l);
> +                             aa_put_label(old);
> +                     } else
> +                             aa_put_label(l);
> +             }
>       }
>       spin_unlock(&unix_sk(sk)->lock);
>  }
> -- 
> 2.51.0
> 
> 
> 
> 


Reply via email to