Re: [PATCH V2] selinux: access sid under READ/WRITE_ONCE
From: Stephen Smalley
Date: Tue Mar 11 2025 - 11:22:39 EST
On Mon, Mar 10, 2025 at 8:03 PM Edward Adam Davis <eadavis@xxxxxx> wrote:
>
> syzbot reported a data-race in selinux_socket_post_create /
> selinux_socket_sock_rcv_skb. [1]
>
> When creating the socket path and receiving the network data packet path,
> effective data access protection is not performed when reading and writing
> the sid, resulting in a race condition.
>
> Use READ_ONCE/WRITE_ONCE to synchronize the two.
>
> [1]
> BUG: KCSAN: data-race in selinux_socket_post_create / selinux_socket_sock_rcv_skb
>
> write to 0xffff88811b989e30 of 4 bytes by task 3803 on cpu 0:
> selinux_socket_post_create+0x1b5/0x2a0 security/selinux/hooks.c:4681
> security_socket_post_create+0x5b/0xa0 security/security.c:4577
> __sock_create+0x35b/0x5a0 net/socket.c:1571
> sock_create net/socket.c:1606 [inline]
> __sys_socket_create net/socket.c:1643 [inline]
> __sys_socket+0xae/0x240 net/socket.c:1690
> __do_sys_socket net/socket.c:1704 [inline]
> __se_sys_socket net/socket.c:1702 [inline]
> __x64_sys_socket+0x3f/0x50 net/socket.c:1702
> x64_sys_call+0x2cf2/0x2dc0 arch/x86/include/generated/asm/syscalls_64.h:42
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xc9/0x1c0 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> read to 0xffff88811b989e30 of 4 bytes by task 3805 on cpu 1:
> selinux_socket_sock_rcv_skb+0x72/0x6a0 security/selinux/hooks.c:5129
> security_sock_rcv_skb+0x3d/0x80 security/security.c:4781
> sk_filter_trim_cap+0xca/0x3c0 net/core/filter.c:151
> sk_filter include/linux/filter.h:1062 [inline]
> sock_queue_rcv_skb_reason+0x28/0xc0 net/core/sock.c:527
> sock_queue_rcv_skb include/net/sock.h:2403 [inline]
> packet_rcv_spkt+0x2f7/0x3b0 net/packet/af_packet.c:1967
> deliver_skb net/core/dev.c:2449 [inline]
> __netif_receive_skb_core+0x48f/0x2350 net/core/dev.c:5737
> __netif_receive_skb_list_core+0x115/0x520 net/core/dev.c:5968
> __netif_receive_skb_list net/core/dev.c:6035 [inline]
> netif_receive_skb_list_internal+0x4e4/0x660 net/core/dev.c:6126
> netif_receive_skb_list+0x31/0x230 net/core/dev.c:6178
> xdp_recv_frames net/bpf/test_run.c:280 [inline]
> xdp_test_run_batch net/bpf/test_run.c:361 [inline]
> bpf_test_run_xdp_live+0xe10/0x1040 net/bpf/test_run.c:390
> bpf_prog_test_run_xdp+0x51d/0x8b0 net/bpf/test_run.c:1316
> bpf_prog_test_run+0x20f/0x3a0 kernel/bpf/syscall.c:4407
> __sys_bpf+0x400/0x7a0 kernel/bpf/syscall.c:5813
> __do_sys_bpf kernel/bpf/syscall.c:5902 [inline]
> __se_sys_bpf kernel/bpf/syscall.c:5900 [inline]
> __x64_sys_bpf+0x43/0x50 kernel/bpf/syscall.c:5900
> x64_sys_call+0x2914/0x2dc0 arch/x86/include/generated/asm/syscalls_64.h:322
> do_syscall_x64 arch/x86/entry/common.c:52 [inline]
> do_syscall_64+0xc9/0x1c0 arch/x86/entry/common.c:83
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
> value changed: 0x00000003 -> 0x00000087
>
> Reported-by: syzbot+00c633585760c05507c3@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=00c633585760c05507c3
> Signed-off-by: Edward Adam Davis <eadavis@xxxxxx>
> ---
> V1 -> V2: replace with READ_ONCE/WRITE_ONCE
Thanks for the patch. Did you audit all other users of sksec->sid to
see if they too should be wrapped with READ_ONCE()/WRITE_ONCE() or are
already safe?
>
> security/selinux/hooks.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 7b867dfec88b..77d2953eaa4d 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4678,7 +4678,7 @@ static int selinux_socket_post_create(struct socket *sock, int family,
> if (sock->sk) {
> sksec = selinux_sock(sock->sk);
> sksec->sclass = sclass;
> - sksec->sid = sid;
> + WRITE_ONCE(sksec->sid, sid);
> /* Allows detection of the first association on this socket */
> if (sksec->sclass == SECCLASS_SCTP_SOCKET)
> sksec->sctp_assoc_state = SCTP_ASSOC_UNSET;
> @@ -5126,7 +5126,7 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
> int err, peerlbl_active, secmark_active;
> struct sk_security_struct *sksec = selinux_sock(sk);
> u16 family = sk->sk_family;
> - u32 sk_sid = sksec->sid;
> + u32 sk_sid = READ_ONCE(sksec->sid);
> struct common_audit_data ad;
> struct lsm_network_audit net;
> char *addrp;
> --
> 2.43.0
>