* [PATCH] apparmor: fix NULL peer label deref for connected SOCK_DGRAM sockets
@ 2026-10-20 17:30 Nik Tsytsarkin
2026-10-21 21:25 ` Serge E. Hallyn
0 siblings, 1 reply; 3+ messages in thread
From: Nik Tsytsarkin @ 2026-10-20 17:30 UTC (permalink / raw)
To: John Johansen, Georgia Garcia, Paul Moore, James Morris,
Serge E. Hallyn, apparmor, linux-security-module, linux-kernel
Cc: Sam Dlinn
apparmor_sk_alloc_security() initializes ctx->peer and
ctx->peer_lastupdate to NULL, and unix_connect_peers() is the only
place they are set. It is called from the unix_stream_connect and
socket_socketpair hooks, but not for SOCK_DGRAM sockets paired with
connect(2): the dgram connect path only goes through the unix_may_send
hook, which does not touch the socket's apparmor context. So after
connect(2) on an AF_UNIX SOCK_DGRAM socket, unix_peer(sk) is set while
ctx->peer stays NULL.
When a confined task later triggers file revalidation on such a
socket (its first write(2) on the fd suffices, with or without a
profile change in between), aa_unix_file_perm() runs the post-check
cache update with peer_sk != NULL and a NULL cached peer label, and
both update paths dereference the NULL label:
- update_peer_ctx() calls aa_label_merge(old, label, ...) with
old == NULL, which reads old->size;
- update_sk_ctx() calls aa_label_is_subset(plabel, old) with
old == NULL, which reads old->size via
__aa_label_next_not_in_set().
The AA_BUG() NULL assertions in those helpers compile out without
CONFIG_SECURITY_APPARMOR_DEBUG_ASSERTS, so production builds oops
under spin_lock(&unix_sk(sk)->lock). An unprivileged confined task
can trigger this whenever its profile mediates unix sockets (complain
mode included). Because the oops happens under the unix sock
spinlock, the dying task then deadlocks against itself in
unix_release_sock() on exit, leaving an unkillable spinning task and
an RCU stall on that CPU (and a full crash with panic_on_oops=1).
BUG: kernel NULL pointer dereference, address: 000000000000004c
RIP: 0010:aa_label_merge+0x63/0x730
Call Trace:
aa_unix_file_perm+0x477/0x800
aa_file_perm+0x434/0x550
common_file_perm+0x58/0x150
security_file_permission+0x22/0x50
vfs_write+0x76/0x410
ksys_write+0xb3/0xd0
Treat a NULL cached peer label as "not initialized yet" in both
update helpers: have update_peer_ctx() adopt the checked label
instead of merging with the missing old one, and have update_sk_ctx()
initialize ctx->peer/ctx->peer_lastupdate when no peer label was
recorded. This lazily completes, at first revalidation, the peer
cross-referencing that unix_connect_peers() performs for stream and
socketpair sockets.
Fixes: c05e705812d1 ("apparmor: add fine grained af_unix mediation")
Assisted-by: LLM
Signed-off-by: Nik Tsytsarkin (nikt@fb.com)
---
security/apparmor/af_unix.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c
index b908e744818c..e802538b0e54 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,7 +700,11 @@ 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 (old)
+ l = aa_label_merge(old, label, GFP_ATOMIC);
+ else
+ /* connected dgram socket, peer ctx was never initialized */
+ l = aa_get_label(label);
if (l) {
if (l != old) {
rcu_assign_pointer(ctx->peer, l);
--
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] apparmor: fix NULL peer label deref for connected SOCK_DGRAM sockets
2026-10-20 17:30 [PATCH] apparmor: fix NULL peer label deref for connected SOCK_DGRAM sockets Nik Tsytsarkin
@ 2026-10-21 21:25 ` Serge E. Hallyn
2026-10-21 22:17 ` Nik Tsytsarkin
0 siblings, 1 reply; 3+ messages in thread
From: Serge E. Hallyn @ 2026-10-21 21:25 UTC (permalink / raw)
To: Nik Tsytsarkin
Cc: John Johansen, Georgia Garcia, Paul Moore, James Morris,
apparmor, linux-security-module, linux-kernel, Sam Dlinn
On Tue, Oct 20, 2026 at 12:30:41PM -0500, Nik Tsytsarkin wrote:
> apparmor_sk_alloc_security() initializes ctx->peer and
> ctx->peer_lastupdate to NULL, and unix_connect_peers() is the only
> place they are set. It is called from the unix_stream_connect and
> socket_socketpair hooks, but not for SOCK_DGRAM sockets paired with
> connect(2): the dgram connect path only goes through the unix_may_send
> hook, which does not touch the socket's apparmor context. So after
> connect(2) on an AF_UNIX SOCK_DGRAM socket, unix_peer(sk) is set while
> ctx->peer stays NULL.
>
> When a confined task later triggers file revalidation on such a
> socket (its first write(2) on the fd suffices, with or without a
> profile change in between), aa_unix_file_perm() runs the post-check
> cache update with peer_sk != NULL and a NULL cached peer label, and
> both update paths dereference the NULL label:
>
> - update_peer_ctx() calls aa_label_merge(old, label, ...) with
> old == NULL, which reads old->size;
> - update_sk_ctx() calls aa_label_is_subset(plabel, old) with
> old == NULL, which reads old->size via
> __aa_label_next_not_in_set().
>
> The AA_BUG() NULL assertions in those helpers compile out without
> CONFIG_SECURITY_APPARMOR_DEBUG_ASSERTS, so production builds oops
> under spin_lock(&unix_sk(sk)->lock). An unprivileged confined task
> can trigger this whenever its profile mediates unix sockets (complain
> mode included). Because the oops happens under the unix sock
> spinlock, the dying task then deadlocks against itself in
> unix_release_sock() on exit, leaving an unkillable spinning task and
> an RCU stall on that CPU (and a full crash with panic_on_oops=1).
>
> BUG: kernel NULL pointer dereference, address: 000000000000004c
> RIP: 0010:aa_label_merge+0x63/0x730
> Call Trace:
> aa_unix_file_perm+0x477/0x800
> aa_file_perm+0x434/0x550
> common_file_perm+0x58/0x150
> security_file_permission+0x22/0x50
> vfs_write+0x76/0x410
> ksys_write+0xb3/0xd0
>
> Treat a NULL cached peer label as "not initialized yet" in both
> update helpers: have update_peer_ctx() adopt the checked label
> instead of merging with the missing old one, and have update_sk_ctx()
> initialize ctx->peer/ctx->peer_lastupdate when no peer label was
> recorded. This lazily completes, at first revalidation, the peer
> cross-referencing that unix_connect_peers() performs for stream and
> socketpair sockets.
>
> Fixes: c05e705812d1 ("apparmor: add fine grained af_unix mediation")
> Assisted-by: LLM
How did you generate this patch? (It looks damaged.)
> Signed-off-by: Nik Tsytsarkin (nikt@fb.com)
> ---
> security/apparmor/af_unix.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c
> index b908e744818c..e802538b0e54 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,7 +700,11 @@ 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 (old)
> + l = aa_label_merge(old, label, GFP_ATOMIC);
> + else
> + /* connected dgram socket, peer ctx was never initialized */
> + l = aa_get_label(label);
> if (l) {
> if (l != old) {
> rcu_assign_pointer(ctx->peer, l);
> --
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] apparmor: fix NULL peer label deref for connected SOCK_DGRAM sockets
2026-10-21 21:25 ` Serge E. Hallyn
@ 2026-10-21 22:17 ` Nik Tsytsarkin
0 siblings, 0 replies; 3+ messages in thread
From: Nik Tsytsarkin @ 2026-10-21 22:17 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: John Johansen, Georgia Garcia, Paul Moore, James Morris,
apparmor, linux-security-module, linux-kernel, Sam Dlinn
Sorry, I messed up the brackets while putting my email in into the
signed off field.
From 4ffed66a07d63db409ad272309fe4946ed758360 Mon Sep 17 00:00:00 2001
From: nikt <nikt@meta.com>
Date: Mon, 19 Oct 2026 15:17:33 -0700
Subject: [PATCH] apparmor: fix NULL peer label deref for connected SOCK_DGRAM
sockets
apparmor_sk_alloc_security() initializes ctx->peer and
ctx->peer_lastupdate to NULL, and unix_connect_peers() is the only
place they are set. It is called from the unix_stream_connect and
socket_socketpair hooks, but not for SOCK_DGRAM sockets paired with
connect(2): the dgram connect path only goes through the unix_may_send
hook, which does not touch the socket's apparmor context. So after
connect(2) on an AF_UNIX SOCK_DGRAM socket, unix_peer(sk) is set while
ctx->peer stays NULL.
When a confined task later triggers file revalidation on such a
socket (its first write(2) on the fd suffices, with or without a
profile change in between), aa_unix_file_perm() runs the post-check
cache update with peer_sk != NULL and a NULL cached peer label, and
both update paths dereference the NULL label:
- update_peer_ctx() calls aa_label_merge(old, label, ...) with
old == NULL, which reads old->size;
- update_sk_ctx() calls aa_label_is_subset(plabel, old) with
old == NULL, which reads old->size via
__aa_label_next_not_in_set().
The AA_BUG() NULL assertions in those helpers compile out without
CONFIG_SECURITY_APPARMOR_DEBUG_ASSERTS, so production builds oops
under spin_lock(&unix_sk(sk)->lock). An unprivileged confined task
can trigger this whenever its profile mediates unix sockets (complain
mode included). Because the oops happens under the unix sock
spinlock, the dying task then deadlocks against itself in
unix_release_sock() on exit, leaving an unkillable spinning task and
an RCU stall on that CPU (and a full crash with panic_on_oops=1).
BUG: kernel NULL pointer dereference, address: 000000000000004c
RIP: 0010:aa_label_merge+0x63/0x730
Call Trace:
aa_unix_file_perm+0x477/0x800
aa_file_perm+0x434/0x550
common_file_perm+0x58/0x150
security_file_permission+0x22/0x50
vfs_write+0x76/0x410
ksys_write+0xb3/0xd0
Treat a NULL cached peer label as "not initialized yet" in both
update helpers: have update_peer_ctx() adopt the checked label
instead of merging with the missing old one, and have update_sk_ctx()
initialize ctx->peer/ctx->peer_lastupdate when no peer label was
recorded. This lazily completes, at first revalidation, the peer
cross-referencing that unix_connect_peers() performs for stream and
socketpair sockets.
Fixes: c05e705812d1 ("apparmor: add fine grained af_unix mediation")
Assisted-by: LLM
Signed-off-by: Nik Tsytsarkin <nikt@meta.com>
---
security/apparmor/af_unix.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c
index b908e744818c..e802538b0e54 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,7 +700,11 @@ 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 (old)
+ l = aa_label_merge(old, label, GFP_ATOMIC);
+ else
+ /* connected dgram socket, peer ctx was never initialized */
+ l = aa_get_label(label);
if (l) {
if (l != old) {
rcu_assign_pointer(ctx->peer, l);
--
2.53.0
On Wed, Oct 21, 2026 at 4:25 PM Serge E. Hallyn <serge@hallyn.com> wrote:
>
> >
> On Tue, Oct 20, 2026 at 12:30:41PM -0500, Nik Tsytsarkin wrote:
> > apparmor_sk_alloc_security() initializes ctx->peer and
> > ctx->peer_lastupdate to NULL, and unix_connect_peers() is the only
> > place they are set. It is called from the unix_stream_connect and
> > socket_socketpair hooks, but not for SOCK_DGRAM sockets paired with
> > connect(2): the dgram connect path only goes through the unix_may_send
> > hook, which does not touch the socket's apparmor context. So after
> > connect(2) on an AF_UNIX SOCK_DGRAM socket, unix_peer(sk) is set while
> > ctx->peer stays NULL.
> >
> > When a confined task later triggers file revalidation on such a
> > socket (its first write(2) on the fd suffices, with or without a
> > profile change in between), aa_unix_file_perm() runs the post-check
> > cache update with peer_sk != NULL and a NULL cached peer label, and
> > both update paths dereference the NULL label:
> >
> > - update_peer_ctx() calls aa_label_merge(old, label, ...) with
> > old == NULL, which reads old->size;
> > - update_sk_ctx() calls aa_label_is_subset(plabel, old) with
> > old == NULL, which reads old->size via
> > __aa_label_next_not_in_set().
> >
> > The AA_BUG() NULL assertions in those helpers compile out without
> > CONFIG_SECURITY_APPARMOR_DEBUG_ASSERTS, so production builds oops
> > under spin_lock(&unix_sk(sk)->lock). An unprivileged confined task
> > can trigger this whenever its profile mediates unix sockets (complain
> > mode included). Because the oops happens under the unix sock
> > spinlock, the dying task then deadlocks against itself in
> > unix_release_sock() on exit, leaving an unkillable spinning task and
> > an RCU stall on that CPU (and a full crash with panic_on_oops=1).
> >
> > BUG: kernel NULL pointer dereference, address: 000000000000004c
> > RIP: 0010:aa_label_merge+0x63/0x730
> > Call Trace:
> > aa_unix_file_perm+0x477/0x800
> > aa_file_perm+0x434/0x550
> > common_file_perm+0x58/0x150
> > security_file_permission+0x22/0x50
> > vfs_write+0x76/0x410
> > ksys_write+0xb3/0xd0
> >
> > Treat a NULL cached peer label as "not initialized yet" in both
> > update helpers: have update_peer_ctx() adopt the checked label
> > instead of merging with the missing old one, and have update_sk_ctx()
> > initialize ctx->peer/ctx->peer_lastupdate when no peer label was
> > recorded. This lazily completes, at first revalidation, the peer
> > cross-referencing that unix_connect_peers() performs for stream and
> > socketpair sockets.
> >
> > Fixes: c05e705812d1 ("apparmor: add fine grained af_unix mediation")
> > Assisted-by: LLM
>
> How did you generate this patch? (It looks damaged.)
>
> > Signed-off-by: Nik Tsytsarkin (nikt@fb.com)
> > ---
> > security/apparmor/af_unix.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/security/apparmor/af_unix.c b/security/apparmor/af_unix.c
> > index b908e744818c..e802538b0e54 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,7 +700,11 @@ 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 (old)
> > + l = aa_label_merge(old, label, GFP_ATOMIC);
> > + else
> > + /* connected dgram socket, peer ctx was never initialized */
> > + l = aa_get_label(label);
> > if (l) {
> > if (l != old) {
> > rcu_assign_pointer(ctx->peer, l);
> > --
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-21 22:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-20 17:30 [PATCH] apparmor: fix NULL peer label deref for connected SOCK_DGRAM sockets Nik Tsytsarkin
2026-10-21 21:25 ` Serge E. Hallyn
2026-10-21 22:17 ` Nik Tsytsarkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®