| Message ID | 20240509081459.2807828-1-leitao@debian.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 540bf24fba16b88c1b3b9353927204b4f1074e25 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] af_unix: Fix data races in unix_release_sock/unix_stream_sendmsg | expand |
From: Breno Leitao <leitao@debian.org> 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/20240508173324.53565-1-kuniyu@amazon.com/ > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Breno Leitao <leitao@debian.org> Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com> 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 >
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: 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!
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) {
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/20240508173324.53565-1-kuniyu@amazon.com/ Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Breno Leitao <leitao@debian.org> --- 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(-)