Linux Security Modules development
 help / color / mirror / Atom feed
* [PATCH] apparmor: fix NULL ctx->peer derefs in unix socket ctx updates
@ 2026-08-24 15:58 Maxime Bélair
  2026-08-24 21:32 ` Aurelien Jarno
  2026-09-30 10:19 ` [apparmor] " Fabian Grünbichler
  0 siblings, 2 replies; 5+ messages in thread
From: Maxime Bélair @ 2026-08-24 15:58 UTC (permalink / raw)
  To: John Johansen, Georgia Garcia
  Cc: apparmor, linux-security-module, Aurelien Jarno,
	Maxime Bélair

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 <aurelien@aurel32.net>
Closes: https://bugs.debian.org/1145111
Cc: stable@vger.kernel.org
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] apparmor: fix NULL ctx->peer derefs in unix socket ctx updates
  2026-08-24 15:58 [PATCH] apparmor: fix NULL ctx->peer derefs in unix socket ctx updates Maxime Bélair
@ 2026-08-24 21:32 ` Aurelien Jarno
  2026-08-31 20:25   ` Aurelien Jarno
  2026-09-30 10:19 ` [apparmor] " Fabian Grünbichler
  1 sibling, 1 reply; 5+ messages in thread
From: Aurelien Jarno @ 2026-08-24 21:32 UTC (permalink / raw)
  To: Maxime Bélair
  Cc: John Johansen, Georgia Garcia, apparmor, linux-security-module

Hi Maxime,

On 2026-08-24 17:58, Maxime Bélair 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 <aurelien@aurel32.net>
> Closes: https://bugs.debian.org/1145111
> Cc: stable@vger.kernel.org
> Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
> ---
>  security/apparmor/af_unix.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)

Thanks a lot for the quick patch. I confirm it fixes the issue I 
reported.

Tested-by: Aurelien Jarno <aurelien@aurel32.net>

Regards
Aurelien

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] apparmor: fix NULL ctx->peer derefs in unix socket ctx updates
  2026-08-24 21:32 ` Aurelien Jarno
@ 2026-08-31 20:25   ` Aurelien Jarno
  2026-09-25 21:41     ` Salvatore Bonaccorso
  0 siblings, 1 reply; 5+ messages in thread
From: Aurelien Jarno @ 2026-08-31 20:25 UTC (permalink / raw)
  To: Maxime Bélair
  Cc: John Johansen, Georgia Garcia, apparmor, linux-security-module

Hi,

On 2026-08-24 23:32, Aurelien Jarno wrote:
> Hi Maxime,
> 
> On 2026-08-24 17:58, Maxime Bélair 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 <aurelien@aurel32.net>
> > Closes: https://bugs.debian.org/1145111
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
> > ---
> >  security/apparmor/af_unix.c | 20 ++++++++++++--------
> >  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> Thanks a lot for the quick patch. I confirm it fixes the issue I 
> reported.
> 
> Tested-by: Aurelien Jarno <aurelien@aurel32.net>

Any news about this patch? The kernel oops can be triggered as a simple 
user, so it would be nice to get it fixed relatively soon.

Thanks
Aurelien

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] apparmor: fix NULL ctx->peer derefs in unix socket ctx updates
  2026-08-31 20:25   ` Aurelien Jarno
@ 2026-09-25 21:41     ` Salvatore Bonaccorso
  0 siblings, 0 replies; 5+ messages in thread
From: Salvatore Bonaccorso @ 2026-09-25 21:41 UTC (permalink / raw)
  To: Aurelien Jarno, Maxime Bélair
  Cc: John Johansen, Georgia Garcia, apparmor, linux-security-module,
	1145111

Hi Maxime,

On Mon, Aug 31, 2026 at 10:25:07PM +0200, Aurelien Jarno wrote:
> Hi,
> 
> On 2026-08-24 23:32, Aurelien Jarno wrote:
> > Hi Maxime,
> > 
> > On 2026-08-24 17:58, Maxime Bélair 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 <aurelien@aurel32.net>
> > > Closes: https://bugs.debian.org/1145111
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
> > > ---
> > >  security/apparmor/af_unix.c | 20 ++++++++++++--------
> > >  1 file changed, 12 insertions(+), 8 deletions(-)
> > 
> > Thanks a lot for the quick patch. I confirm it fixes the issue I 
> > reported.
> > 
> > Tested-by: Aurelien Jarno <aurelien@aurel32.net>
> 
> Any news about this patch? The kernel oops can be triggered as a simple 
> user, so it would be nice to get it fixed relatively soon.

Any news here? As Aurelien has pointed out this can be relatively easy
triggered, so having a fix landing would be great.

Regards,
Salvatore

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [apparmor] [PATCH] apparmor: fix NULL ctx->peer derefs in unix socket ctx updates
  2026-08-24 15:58 [PATCH] apparmor: fix NULL ctx->peer derefs in unix socket ctx updates Maxime Bélair
  2026-08-24 21:32 ` Aurelien Jarno
@ 2026-09-30 10:19 ` Fabian Grünbichler
  1 sibling, 0 replies; 5+ messages in thread
From: Fabian Grünbichler @ 2026-09-30 10:19 UTC (permalink / raw)
  To: Georgia Garcia, John Johansen, Maxime Bélair
  Cc: apparmor, Aurelien Jarno, linux-security-module

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 <aurelien@aurel32.net>
> Closes: https://bugs.debian.org/1145111
> Cc: stable@vger.kernel.org
> Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>

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 <f.gruenbichler@proxmox.com>

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-30 10:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-24 15:58 [PATCH] apparmor: fix NULL ctx->peer derefs in unix socket ctx updates Maxime Bélair
2026-08-24 21:32 ` Aurelien Jarno
2026-08-31 20:25   ` Aurelien Jarno
2026-09-25 21:41     ` Salvatore Bonaccorso
2026-09-30 10:19 ` [apparmor] " Fabian Grünbichler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox