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