2024-05-09 08:15:23

by Breno Leitao

[permalink] [raw]
Subject: [PATCH net v2] af_unix: Fix data races in unix_release_sock/unix_stream_sendmsg

A data-race condition has been identified in af_unix. In one data path,
the write function unix_release_sock() atomically writes to
sk->sk_shutdown using WRITE_ONCE. However, on the reader side,
unix_stream_sendmsg() does not read it atomically. Consequently, this
issue is causing the following KCSAN splat to occur:

BUG: KCSAN: data-race in unix_release_sock / unix_stream_sendmsg

write (marked) to 0xffff88867256ddbb of 1 bytes by task 7270 on cpu 28:
unix_release_sock (net/unix/af_unix.c:640)
unix_release (net/unix/af_unix.c:1050)
sock_close (net/socket.c:659 net/socket.c:1421)
__fput (fs/file_table.c:422)
__fput_sync (fs/file_table.c:508)
__se_sys_close (fs/open.c:1559 fs/open.c:1541)
__x64_sys_close (fs/open.c:1541)
x64_sys_call (arch/x86/entry/syscall_64.c:33)
do_syscall_64 (arch/x86/entry/common.c:?)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)

read to 0xffff88867256ddbb of 1 bytes by task 989 on cpu 14:
unix_stream_sendmsg (net/unix/af_unix.c:2273)
__sock_sendmsg (net/socket.c:730 net/socket.c:745)
____sys_sendmsg (net/socket.c:2584)
__sys_sendmmsg (net/socket.c:2638 net/socket.c:2724)
__x64_sys_sendmmsg (net/socket.c:2753 net/socket.c:2750 net/socket.c:2750)
x64_sys_call (arch/x86/entry/syscall_64.c:33)
do_syscall_64 (arch/x86/entry/common.c:?)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)

value changed: 0x01 -> 0x03

The line numbers are related to commit dd5a440a31fa ("Linux 6.9-rc7").

Commit e1d09c2c2f57 ("af_unix: Fix data races around sk->sk_shutdown.")
addressed a comparable issue in the past regarding sk->sk_shutdown.
However, it overlooked resolving this particular data path.
This patch only offending unix_stream_sendmsg() function, since the
other reads seem to be protected by unix_state_lock() as discussed in
Link: https://lore.kernel.org/all/[email protected]/

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Breno Leitao <[email protected]>
---
Changelog:

v2:
* Only fix the usecase reported by KCSAN
* Targeting net instead of net-next
---
net/unix/af_unix.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index dc1651541723..fa906ec5e657 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2224,7 +2224,7 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
goto out_err;
}

- if (sk->sk_shutdown & SEND_SHUTDOWN)
+ if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN)
goto pipe_err;

while (sent < len) {
--
2.43.0



2024-05-10 04:50:19

by Kuniyuki Iwashima

[permalink] [raw]
Subject: Re: [PATCH net v2] af_unix: Fix data races in unix_release_sock/unix_stream_sendmsg

From: Breno Leitao <[email protected]>
Date: Thu, 9 May 2024 01:14:46 -0700
> A data-race condition has been identified in af_unix. In one data path,
> the write function unix_release_sock() atomically writes to
> sk->sk_shutdown using WRITE_ONCE. However, on the reader side,
> unix_stream_sendmsg() does not read it atomically. Consequently, this
> issue is causing the following KCSAN splat to occur:
>
> BUG: KCSAN: data-race in unix_release_sock / unix_stream_sendmsg
>
> write (marked) to 0xffff88867256ddbb of 1 bytes by task 7270 on cpu 28:
> unix_release_sock (net/unix/af_unix.c:640)
> unix_release (net/unix/af_unix.c:1050)
> sock_close (net/socket.c:659 net/socket.c:1421)
> __fput (fs/file_table.c:422)
> __fput_sync (fs/file_table.c:508)
> __se_sys_close (fs/open.c:1559 fs/open.c:1541)
> __x64_sys_close (fs/open.c:1541)
> x64_sys_call (arch/x86/entry/syscall_64.c:33)
> do_syscall_64 (arch/x86/entry/common.c:?)
> entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
>
> read to 0xffff88867256ddbb of 1 bytes by task 989 on cpu 14:
> unix_stream_sendmsg (net/unix/af_unix.c:2273)
> __sock_sendmsg (net/socket.c:730 net/socket.c:745)
> ____sys_sendmsg (net/socket.c:2584)
> __sys_sendmmsg (net/socket.c:2638 net/socket.c:2724)
> __x64_sys_sendmmsg (net/socket.c:2753 net/socket.c:2750 net/socket.c:2750)
> x64_sys_call (arch/x86/entry/syscall_64.c:33)
> do_syscall_64 (arch/x86/entry/common.c:?)
> entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
>
> value changed: 0x01 -> 0x03
>
> The line numbers are related to commit dd5a440a31fa ("Linux 6.9-rc7").
>
> Commit e1d09c2c2f57 ("af_unix: Fix data races around sk->sk_shutdown.")
> addressed a comparable issue in the past regarding sk->sk_shutdown.
> However, it overlooked resolving this particular data path.
> This patch only offending unix_stream_sendmsg() function, since the
> other reads seem to be protected by unix_state_lock() as discussed in
> Link: https://lore.kernel.org/all/[email protected]/
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Breno Leitao <[email protected]>

Reviewed-by: Kuniyuki Iwashima <[email protected]>

Thanks!

> ---
> Changelog:
>
> v2:
> * Only fix the usecase reported by KCSAN
> * Targeting net instead of net-next
> ---
> net/unix/af_unix.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index dc1651541723..fa906ec5e657 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -2224,7 +2224,7 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
> goto out_err;
> }
>
> - if (sk->sk_shutdown & SEND_SHUTDOWN)
> + if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN)
> goto pipe_err;
>
> while (sent < len) {
> --
> 2.43.0
>

2024-05-11 02:31:04

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net v2] af_unix: Fix data races in unix_release_sock/unix_stream_sendmsg

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

On Thu, 9 May 2024 01:14:46 -0700 you wrote:
> A data-race condition has been identified in af_unix. In one data path,
> the write function unix_release_sock() atomically writes to
> sk->sk_shutdown using WRITE_ONCE. However, on the reader side,
> unix_stream_sendmsg() does not read it atomically. Consequently, this
> issue is causing the following KCSAN splat to occur:
>
> BUG: KCSAN: data-race in unix_release_sock / unix_stream_sendmsg
>
> [...]

Here is the summary with links:
- [net,v2] af_unix: Fix data races in unix_release_sock/unix_stream_sendmsg
https://git.kernel.org/netdev/net/c/540bf24fba16

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html