security/selinux/hooks.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=00c633585760c05507c3
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
V1 -> V2: replace with READ_ONCE/WRITE_ONCE
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
On Mon, Mar 10, 2025 at 8:03 PM Edward Adam Davis <eadavis@qq.com> 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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=00c633585760c05507c3
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> 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
>
On Tue, 11 Mar 2025 11:19:49 -0400, Stephen Smalley wrote: > > Reported-by: syzbot+00c633585760c05507c3@syzkaller.appspotmail.com > > Closes: https://syzkaller.appspot.com/bug?extid=00c633585760c05507c3 > > Signed-off-by: Edward Adam Davis <eadavis@qq.com> > > --- > > 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? This fix is only for the issues reported by syzbot+00c633585760c05507c3. I have confirmed that there are many contexts related to "sksec->sid" (at least 29). I am not familiar with selinux, and it is unnecessary to do excessive fixes before syzbot reports other issues. Maybe the subject of my patch is not appropriate, and it may be more appropriate to adjust it to "selinux: fix data race in socket create and receive skb". BR, Edward
On Tue, Mar 11, 2025 at 9:05 PM Edward Adam Davis <eadavis@qq.com> wrote: > > On Tue, 11 Mar 2025 11:19:49 -0400, Stephen Smalley wrote: > > > Reported-by: syzbot+00c633585760c05507c3@syzkaller.appspotmail.com > > > Closes: https://syzkaller.appspot.com/bug?extid=00c633585760c05507c3 > > > Signed-off-by: Edward Adam Davis <eadavis@qq.com> > > > --- > > > 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? > This fix is only for the issues reported by syzbot+00c633585760c05507c3. > I have confirmed that there are many contexts related to "sksec->sid" (at > least 29). I am not familiar with selinux, and it is unnecessary to do > excessive fixes before syzbot reports other issues. > > Maybe the subject of my patch is not appropriate, and it may be more > appropriate to adjust it to "selinux: fix data race in socket create and > receive skb". We don't want to just silence warnings but rather identify and fix root causes, and do so comprehensively. Looking more closely at the syzbot report, it appears that a sock that initially has SID 3 (aka SECINITSID_UNLABELED) is being assigned a specific SID via socket_post_create at a point where it might already be receiving packets. That seems like it requires a more general fix to ensure that the sock is correctly labeled before it can start receiving packets.
On Wed, Mar 12, 2025 at 9:23 AM Stephen Smalley <stephen.smalley.work@gmail.com> wrote: > > On Tue, Mar 11, 2025 at 9:05 PM Edward Adam Davis <eadavis@qq.com> wrote: > > > > On Tue, 11 Mar 2025 11:19:49 -0400, Stephen Smalley wrote: > > > > Reported-by: syzbot+00c633585760c05507c3@syzkaller.appspotmail.com > > > > Closes: https://syzkaller.appspot.com/bug?extid=00c633585760c05507c3 > > > > Signed-off-by: Edward Adam Davis <eadavis@qq.com> > > > > --- > > > > 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? > > This fix is only for the issues reported by syzbot+00c633585760c05507c3. > > I have confirmed that there are many contexts related to "sksec->sid" (at > > least 29). I am not familiar with selinux, and it is unnecessary to do > > excessive fixes before syzbot reports other issues. > > > > Maybe the subject of my patch is not appropriate, and it may be more > > appropriate to adjust it to "selinux: fix data race in socket create and > > receive skb". > > We don't want to just silence warnings but rather identify and fix > root causes, and do so comprehensively. > Looking more closely at the syzbot report, it appears that a sock that > initially has SID 3 (aka SECINITSID_UNLABELED) is being assigned a > specific SID via socket_post_create at a point where it might already > be receiving packets. > That seems like it requires a more general fix to ensure that the sock > is correctly labeled before it can start receiving packets. There is a window in __sock_create() where the socket is created and "alive" in the network stack, but before security_socket_post_create() is called to fully initialize/label the socket (look between pf->create() and the LSM call in __sock_create()). Looking quickly, I *think* it may be as simple as doing the read/write_once() accessors for the sksec->sid, but I didn't dig into the NetLabel and XFRM aspects of the code paths. I suspect they are okay due to how they work, but I'm not going to be surprised if there is an issue lurking there. We could possibly solve this generically by introducing a sksec->initialized field, similar to inodes, although we would have no way to properly initialize the sksec in selinux_socket_sock_rcv_skb() if we hit the uninitialized case so we would need to decide how to handle that. I worry that dropping packets in that case could negatively impact stream connections that need to go through a multi-step handshake process. Maybe we could capture the creator's label in selinux_sk_alloc_security(), at least in some cases (?), but this would need a lot of investigation to see if that works properly in all the cases (I worry it doesn't). -- paul-moore.com
© 2016 - 2026 Red Hat, Inc.