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@xxxxxxxxx>

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@xxxxxxxxxxx>

> ---
>  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
> 
> 
> 
>