Re: [PATCH] apparmor: fix NULL ctx->peer derefs in unix socket ctx updates

Aurelien Jarno <[email protected]>
Newsgroups org.kernel.vger.linux-security-module
Message-ID <[email protected]>
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 <[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(-)

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

Tested-by: Aurelien Jarno <[email protected]>

Regards
Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
[email protected]                     http://aurel32.net
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.