[PATCH] apparmor: handle NULL peer label in AF_UNIX context updates

[PATCH] apparmor: handle NULL peer label in AF_UNIX context updates

[Maciek Borzecki]

When a confined process performs the first file permission revalidation on
an AF_UNIX socket, both update_sk_ctx() and update_peer_ctx() can be called
before the socket's peer label cache (ctx->peer) has been populated. In
those cases they pass NULL as a label argument to aa_label_is_subset() or
aa_label_merge(), which immediately dereferences label->size and causes an
Oops:

  RIP: __aa_label_next_not_in_set+0xd/0x110
  CR2: 000000000000004c
  Call Trace:
    aa_label_is_subset
    aa_unix_file_perm
    aa_file_perm
    apparmor_file_permission
    security_file_permission
    rw_verify_area
    vfs_write
    ksys_write

Observed on Arch Linux kernels 7.2.4-arch1-2 and 7.2.6-arch2-1, triggered
by snapd unit tests writing to a connected AF_UNIX socket. The issue
persists across those stable updates.

Why this can happen:

* apparmor_socket_socketpair() calls unix_connect_peers(), which pre-
  populates ctx->peer for AF_UNIX socketpairs. Because of that, simple
  socketpair() tests never exercise the NULL-peer path.

* For an AF_UNIX SOCK_DGRAM socket that is connected explicitly (or via
  any path that does not go through unix_connect_peers()), ctx->peer is
  still NULL the first time aa_unix_file_perm() runs. The permission check
  succeeds, then update_peer_ctx() tries to merge label into ctx->peer and
  update_sk_ctx() calls aa_label_is_subset(plabel, ctx->peer). Both
  dereference NULL.

The following reproducer profile demonstrates the bug:

    profile aa_unix_server /path/to/repro {
        #include <abstractions/base>

        change_profile -> aa_unix_client,
        file,
        unix,
        /path/to/repro rmix,
    }

    profile aa_unix_client {
        #include <abstractions/base>

        file,
        unix,
    }

Run a program that binds an abstract AF_UNIX SOCK_DGRAM socket under the
server profile, then forks a child that changes into the aa_unix_client
profile, connects the DGRAM socket to the abstract address, and writes().
The write() path reaches the context-update code with ctx->peer == NULL and
oopses.

Fix all spots in update_sk_ctx() and update_peer_ctx() that can see a NULL
ctx->peer:

1. update_sk_ctx() RCU check: only call aa_label_is_subset() when
   ctx->peer is non-NULL. If it is still NULL, an update is required.

2. update_sk_ctx() spin-locked section: treat a NULL old peer label as
   "plabel is a superset", i.e. populate ctx->peer with plabel.

3. update_peer_ctx(): if ctx->peer is currently NULL, just store the new
   label directly instead of trying to merge with NULL.

Fixes: 88fec3526e84 ("apparmor: make sure unix socket labeling is correctly updated.")
Signed-off-by: Maciek Borzecki <maciek.borzecki@gmail.com>
---
 security/apparmor/af_unix.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c
index b908e744818c9bfef1638c6abadec75c241dc9b9..4762b70cdc43b5dac87f079067eca0ca74d5dd52 100644
--- a/security/apparmor/af_unix.c
+++ b/security/apparmor/af_unix.c
@@ -659,7 +659,8 @@ static void update_sk_ctx(struct sock *sk, struct aa_label *label,
 	rcu_read_lock();
 	update_sk = (plabel &&
 		     (plabel != rcu_access_pointer(ctx->peer_lastupdate) ||
-		      !aa_label_is_subset(plabel, rcu_dereference(ctx->peer)))) ||
+		      (rcu_access_pointer(ctx->peer) &&
+		       !aa_label_is_subset(plabel, rcu_dereference(ctx->peer))))) ||
 	  !__aa_subj_label_is_cached(label, rcu_dereference(ctx->label));
 	rcu_read_unlock();
 	if (!update_sk)
@@ -682,7 +683,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 +701,18 @@ 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.55.0

Re: [PATCH] apparmor: handle NULL peer label in AF_UNIX context updates

[Aurelien Jarno]

Hi,

On 2026-09-17 09:15, Maciek Borzecki wrote:
> When a confined process performs the first file permission revalidation on
> an AF_UNIX socket, both update_sk_ctx() and update_peer_ctx() can be called
> before the socket's peer label cache (ctx->peer) has been populated. In
> those cases they pass NULL as a label argument to aa_label_is_subset() or
> aa_label_merge(), which immediately dereferences label->size and causes an
> Oops:
> 
>   RIP: __aa_label_next_not_in_set+0xd/0x110
>   CR2: 000000000000004c
>   Call Trace:
>     aa_label_is_subset
>     aa_unix_file_perm
>     aa_file_perm
>     apparmor_file_permission
>     security_file_permission
>     rw_verify_area
>     vfs_write
>     ksys_write
> 
> Observed on Arch Linux kernels 7.2.4-arch1-2 and 7.2.6-arch2-1, triggered
> by snapd unit tests writing to a connected AF_UNIX socket. The issue
> persists across those stable updates.
> 

It appears to be the same issue as reported here:
https://bugs.debian.org/1145111

And a patch that looks similar to yours has already been posted:
https://lore.kernel.org/20260824155822.9214-1-maxime.belair@canonical.com

As it seems you have looked at the bug in details, you can maybe comment 
it and add a Tested-by: and/or Reviewed-by:, that might help to make 
things progress.

Regards
Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                     http://aurel32.net

Re: [PATCH] apparmor: handle NULL peer label in AF_UNIX context updates

[Fabian Grünbichler]

On September 17, 2026 9:15 am, Maciek Borzecki wrote:
> When a confined process performs the first file permission revalidation on
> an AF_UNIX socket, both update_sk_ctx() and update_peer_ctx() can be called
> before the socket's peer label cache (ctx->peer) has been populated. In
> those cases they pass NULL as a label argument to aa_label_is_subset() or
> aa_label_merge(), which immediately dereferences label->size and causes an
> Oops:
> 
>   RIP: __aa_label_next_not_in_set+0xd/0x110
>   CR2: 000000000000004c
>   Call Trace:
>     aa_label_is_subset
>     aa_unix_file_perm
>     aa_file_perm
>     apparmor_file_permission
>     security_file_permission
>     rw_verify_area
>     vfs_write
>     ksys_write
> 
> Observed on Arch Linux kernels 7.2.4-arch1-2 and 7.2.6-arch2-1, triggered
> by snapd unit tests writing to a connected AF_UNIX socket. The issue
> persists across those stable updates.
> 
> Why this can happen:
> 
> * apparmor_socket_socketpair() calls unix_connect_peers(), which pre-
>   populates ctx->peer for AF_UNIX socketpairs. Because of that, simple
>   socketpair() tests never exercise the NULL-peer path.
> 
> * For an AF_UNIX SOCK_DGRAM socket that is connected explicitly (or via
>   any path that does not go through unix_connect_peers()), ctx->peer is
>   still NULL the first time aa_unix_file_perm() runs. The permission check
>   succeeds, then update_peer_ctx() tries to merge label into ctx->peer and
>   update_sk_ctx() calls aa_label_is_subset(plabel, ctx->peer). Both
>   dereference NULL.
> 
> The following reproducer profile demonstrates the bug:
> 
>     profile aa_unix_server /path/to/repro {
>         #include <abstractions/base>
> 
>         change_profile -> aa_unix_client,
>         file,
>         unix,
>         /path/to/repro rmix,
>     }
> 
>     profile aa_unix_client {
>         #include <abstractions/base>
> 
>         file,
>         unix,
>     }
> 
> Run a program that binds an abstract AF_UNIX SOCK_DGRAM socket under the
> server profile, then forks a child that changes into the aa_unix_client
> profile, connects the DGRAM socket to the abstract address, and writes().
> The write() path reaches the context-update code with ctx->peer == NULL and
> oopses.
> 
> Fix all spots in update_sk_ctx() and update_peer_ctx() that can see a NULL
> ctx->peer:
> 
> 1. update_sk_ctx() RCU check: only call aa_label_is_subset() when
>    ctx->peer is non-NULL. If it is still NULL, an update is required.
> 
> 2. update_sk_ctx() spin-locked section: treat a NULL old peer label as
>    "plabel is a superset", i.e. populate ctx->peer with plabel.
> 
> 3. update_peer_ctx(): if ctx->peer is currently NULL, just store the new
>    label directly instead of trying to merge with NULL.
> 
> Fixes: 88fec3526e84 ("apparmor: make sure unix socket labeling is correctly updated.")
> Signed-off-by: Maciek Borzecki <maciek.borzecki@gmail.com>

The oops triggers during execution of rustc's testsuite on a system with
AA enabled. This is identical modulo the first hunk to the other, older,
also not yet applied patch referenced by Aurelien. Both patches prevent
the oops triggered by rustc's testsuite.

Tested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>

> ---
>  security/apparmor/af_unix.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c
> index b908e744818c9bfef1638c6abadec75c241dc9b9..4762b70cdc43b5dac87f079067eca0ca74d5dd52 100644
> --- a/security/apparmor/af_unix.c
> +++ b/security/apparmor/af_unix.c
> @@ -659,7 +659,8 @@ static void update_sk_ctx(struct sock *sk, struct aa_label *label,
>  	rcu_read_lock();
>  	update_sk = (plabel &&
>  		     (plabel != rcu_access_pointer(ctx->peer_lastupdate) ||
> -		      !aa_label_is_subset(plabel, rcu_dereference(ctx->peer)))) ||
> +		      (rcu_access_pointer(ctx->peer) &&
> +		       !aa_label_is_subset(plabel, rcu_dereference(ctx->peer))))) ||
>  	  !__aa_subj_label_is_cached(label, rcu_dereference(ctx->label));
>  	rcu_read_unlock();
>  	if (!update_sk)
> @@ -682,7 +683,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 +701,18 @@ 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.55.0
> 
> 
> 
>