1. Patchew
  2. linux
  3. View series

[PATCH] selinux: read and write sid under lock

Edward Adam Davis posted 1 patch 11 months ago 
security/selinux/hooks.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
[PATCH] selinux: read and write sid under lock
Posted by Edward Adam Davis 11 months ago 
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.

Add a lock 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>
---
 security/selinux/hooks.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 7b867dfec88b..ea5d0273f9d5 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4677,8 +4677,10 @@ static int selinux_socket_post_create(struct socket *sock, int family,
 
 	if (sock->sk) {
 		sksec = selinux_sock(sock->sk);
+		spin_lock(&sksec->lock);
 		sksec->sclass = sclass;
 		sksec->sid = sid;
+		spin_unlock(&sksec->lock);
 		/* Allows detection of the first association on this socket */
 		if (sksec->sclass == SECCLASS_SCTP_SOCKET)
 			sksec->sctp_assoc_state = SCTP_ASSOC_UNSET;
@@ -5126,7 +5128,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;
 	struct common_audit_data ad;
 	struct lsm_network_audit net;
 	char *addrp;
@@ -5155,6 +5157,9 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
 	if (err)
 		return err;
 
+	spin_lock(&sksec->lock);
+	sk_sid = sksec->sid;
+	spin_unlock(&sksec->lock);
 	if (peerlbl_active) {
 		u32 peer_sid;
 
-- 
2.43.0
Re: [PATCH] selinux: read and write sid under lock
Posted by Stephen Smalley 11 months ago 
On Sat, Mar 8, 2025 at 11:55 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.
>
> Add a lock 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>
> ---
>  security/selinux/hooks.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 7b867dfec88b..ea5d0273f9d5 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4677,8 +4677,10 @@ static int selinux_socket_post_create(struct socket *sock, int family,
>
>         if (sock->sk) {
>                 sksec = selinux_sock(sock->sk);
> +               spin_lock(&sksec->lock);

You didn't include the diff that adds this lock field to
sk_security_struct, but aside from that, I would suggest something
lighter-weight like READ_ONCE/WRITE_ONCE if possible.

>                 sksec->sclass = sclass;
>                 sksec->sid = sid;
> +               spin_unlock(&sksec->lock);
>                 /* Allows detection of the first association on this socket */
>                 if (sksec->sclass == SECCLASS_SCTP_SOCKET)
>                         sksec->sctp_assoc_state = SCTP_ASSOC_UNSET;
> @@ -5126,7 +5128,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;
>         struct common_audit_data ad;
>         struct lsm_network_audit net;
>         char *addrp;
> @@ -5155,6 +5157,9 @@ static int selinux_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
>         if (err)
>                 return err;
>
> +       spin_lock(&sksec->lock);

If you retain this as a spinlock, then I think you need the irq-safe
version lock/unlock calls in this hook due to some callers.

> +       sk_sid = sksec->sid;
> +       spin_unlock(&sksec->lock);
>         if (peerlbl_active) {
>                 u32 peer_sid;
>
> --
> 2.43.0
>
Re: [PATCH] selinux: read and write sid under lock
Posted by Paul Moore 11 months ago 
On Mon, Mar 10, 2025 at 3:53 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> On Sat, Mar 8, 2025 at 11:55 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.
> >
> > Add a lock to synchronize the two.

...

> > Reported-by: syzbot+00c633585760c05507c3@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=00c633585760c05507c3
> > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > ---
> >  security/selinux/hooks.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 7b867dfec88b..ea5d0273f9d5 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -4677,8 +4677,10 @@ static int selinux_socket_post_create(struct socket *sock, int family,
> >
> >         if (sock->sk) {
> >                 sksec = selinux_sock(sock->sk);
> > +               spin_lock(&sksec->lock);
>
> You didn't include the diff that adds this lock field to
> sk_security_struct, but aside from that, I would suggest something
> lighter-weight like READ_ONCE/WRITE_ONCE if possible.

Yes, please don't add a spinlock to something that is potentially
going to be hit on every packet entering the system.

-- 
paul-moore.com
[PATCH V2] selinux: access sid under READ/WRITE_ONCE
Posted by Edward Adam Davis 11 months ago 
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
Re: [PATCH V2] selinux: access sid under READ/WRITE_ONCE
Posted by Stephen Smalley 11 months ago 
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
>
Re: [PATCH V2] selinux: access sid under READ/WRITE_ONCE
Posted by Edward Adam Davis 11 months ago 
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
Re: [PATCH V2] selinux: access sid under READ/WRITE_ONCE
Posted by Stephen Smalley 11 months ago 
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.
Re: [PATCH V2] selinux: access sid under READ/WRITE_ONCE
Posted by Paul Moore 10 months, 4 weeks ago 
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
Re: [PATCH] selinux: read and write sid under lock
Posted by kernel test robot 11 months ago 
Hi Edward,

kernel test robot noticed the following build errors:

[auto build test ERROR on pcmoore-selinux/next]
[also build test ERROR on linus/master v6.14-rc6 next-20250307]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Edward-Adam-Davis/selinux-read-and-write-sid-under-lock/20250309-130846
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git next
patch link:    https://lore.kernel.org/r/tencent_0BEE86CD3878D26D402DDD6F949484E96E0A%40qq.com
patch subject: [PATCH] selinux: read and write sid under lock
config: i386-defconfig (https://download.01.org/0day-ci/archive/20250310/202503101039.wURTMnYj-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250310/202503101039.wURTMnYj-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503101039.wURTMnYj-lkp@intel.com/

All errors (new ones prefixed by >>):

>> security/selinux/hooks.c:4718:21: error: no member named 'lock' in 'struct sk_security_struct'
    4718 |                 spin_lock(&sksec->lock);
         |                            ~~~~~  ^
   security/selinux/hooks.c:4721:23: error: no member named 'lock' in 'struct sk_security_struct'
    4721 |                 spin_unlock(&sksec->lock);
         |                              ~~~~~  ^
   security/selinux/hooks.c:5198:20: error: no member named 'lock' in 'struct sk_security_struct'
    5198 |         spin_lock(&sksec->lock);
         |                    ~~~~~  ^
   security/selinux/hooks.c:5200:22: error: no member named 'lock' in 'struct sk_security_struct'
    5200 |         spin_unlock(&sksec->lock);
         |                      ~~~~~  ^
   4 errors generated.


vim +4718 security/selinux/hooks.c

  4695	
  4696	static int selinux_socket_post_create(struct socket *sock, int family,
  4697					      int type, int protocol, int kern)
  4698	{
  4699		const struct task_security_struct *tsec = selinux_cred(current_cred());
  4700		struct inode_security_struct *isec = inode_security_novalidate(SOCK_INODE(sock));
  4701		struct sk_security_struct *sksec;
  4702		u16 sclass = socket_type_to_security_class(family, type, protocol);
  4703		u32 sid = SECINITSID_KERNEL;
  4704		int err = 0;
  4705	
  4706		if (!kern) {
  4707			err = socket_sockcreate_sid(tsec, sclass, &sid);
  4708			if (err)
  4709				return err;
  4710		}
  4711	
  4712		isec->sclass = sclass;
  4713		isec->sid = sid;
  4714		isec->initialized = LABEL_INITIALIZED;
  4715	
  4716		if (sock->sk) {
  4717			sksec = selinux_sock(sock->sk);
> 4718			spin_lock(&sksec->lock);
  4719			sksec->sclass = sclass;
  4720			sksec->sid = sid;
  4721			spin_unlock(&sksec->lock);
  4722			/* Allows detection of the first association on this socket */
  4723			if (sksec->sclass == SECCLASS_SCTP_SOCKET)
  4724				sksec->sctp_assoc_state = SCTP_ASSOC_UNSET;
  4725	
  4726			err = selinux_netlbl_socket_post_create(sock->sk, family);
  4727		}
  4728	
  4729		return err;
  4730	}
  4731	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] selinux: read and write sid under lock
Posted by kernel test robot 11 months ago 
Hi Edward,

kernel test robot noticed the following build errors:

[auto build test ERROR on pcmoore-selinux/next]
[also build test ERROR on linus/master v6.14-rc5 next-20250307]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Edward-Adam-Davis/selinux-read-and-write-sid-under-lock/20250309-130846
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git next
patch link:    https://lore.kernel.org/r/tencent_0BEE86CD3878D26D402DDD6F949484E96E0A%40qq.com
patch subject: [PATCH] selinux: read and write sid under lock
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20250310/202503100821.PtEmEm7K-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250310/202503100821.PtEmEm7K-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503100821.PtEmEm7K-lkp@intel.com/

All errors (new ones prefixed by >>):

   security/selinux/hooks.c: In function 'selinux_socket_post_create':
>> security/selinux/hooks.c:4718:33: error: 'struct sk_security_struct' has no member named 'lock'
    4718 |                 spin_lock(&sksec->lock);
         |                                 ^~
   security/selinux/hooks.c:4721:35: error: 'struct sk_security_struct' has no member named 'lock'
    4721 |                 spin_unlock(&sksec->lock);
         |                                   ^~
   security/selinux/hooks.c: In function 'selinux_socket_sock_rcv_skb':
   security/selinux/hooks.c:5198:25: error: 'struct sk_security_struct' has no member named 'lock'
    5198 |         spin_lock(&sksec->lock);
         |                         ^~
   security/selinux/hooks.c:5200:27: error: 'struct sk_security_struct' has no member named 'lock'
    5200 |         spin_unlock(&sksec->lock);
         |                           ^~


vim +4718 security/selinux/hooks.c

  4695	
  4696	static int selinux_socket_post_create(struct socket *sock, int family,
  4697					      int type, int protocol, int kern)
  4698	{
  4699		const struct task_security_struct *tsec = selinux_cred(current_cred());
  4700		struct inode_security_struct *isec = inode_security_novalidate(SOCK_INODE(sock));
  4701		struct sk_security_struct *sksec;
  4702		u16 sclass = socket_type_to_security_class(family, type, protocol);
  4703		u32 sid = SECINITSID_KERNEL;
  4704		int err = 0;
  4705	
  4706		if (!kern) {
  4707			err = socket_sockcreate_sid(tsec, sclass, &sid);
  4708			if (err)
  4709				return err;
  4710		}
  4711	
  4712		isec->sclass = sclass;
  4713		isec->sid = sid;
  4714		isec->initialized = LABEL_INITIALIZED;
  4715	
  4716		if (sock->sk) {
  4717			sksec = selinux_sock(sock->sk);
> 4718			spin_lock(&sksec->lock);
  4719			sksec->sclass = sclass;
  4720			sksec->sid = sid;
  4721			spin_unlock(&sksec->lock);
  4722			/* Allows detection of the first association on this socket */
  4723			if (sksec->sclass == SECCLASS_SCTP_SOCKET)
  4724				sksec->sctp_assoc_state = SCTP_ASSOC_UNSET;
  4725	
  4726			err = selinux_netlbl_socket_post_create(sock->sk, family);
  4727		}
  4728	
  4729		return err;
  4730	}
  4731	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.