[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240508111749.2386649-1-leitao@debian.org>
Date: Wed, 8 May 2024 04:17:45 -0700
From: Breno Leitao <leitao@...ian.org>
To: kuniyu@...zon.com,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org,
horms@...nel.org,
paulmck@...nel.org,
David Howells <dhowells@...hat.com>,
Alexander Mikhalitsyn <alexander@...alicyn.com>,
John Fastabend <john.fastabend@...il.com>,
Daan De Meyer <daan.j.demeyer@...il.com>,
linux-kernel@...r.kernel.org (open list)
Subject: [PATCH net-next] 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.
To prevent potential race conditions in the future, all read accesses to
sk->sk_shutdown in af_unix need be marked with READ_ONCE(). Although
there are additional reads in other->sk_shutdown without atomic reads,
I'm excluding them as I'm uncertain about their potential parallel
execution.
Signed-off-by: Breno Leitao <leitao@...ian.org>
---
net/unix/af_unix.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 9a6ad5974dff..74795e6d13c6 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2270,7 +2270,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) {
@@ -2446,7 +2446,7 @@ int __unix_dgram_recvmsg(struct sock *sk, struct msghdr *msg, size_t size,
unix_state_lock(sk);
/* Signal EOF on disconnected non-blocking SEQPACKET socket. */
if (sk->sk_type == SOCK_SEQPACKET && err == -EAGAIN &&
- (sk->sk_shutdown & RCV_SHUTDOWN))
+ (READ_ONCE(sk->sk_shutdown) & RCV_SHUTDOWN))
err = 0;
unix_state_unlock(sk);
goto out;
@@ -2566,7 +2566,7 @@ static long unix_stream_data_wait(struct sock *sk, long timeo,
if (tail != last ||
(tail && tail->len != last_len) ||
sk->sk_err ||
- (sk->sk_shutdown & RCV_SHUTDOWN) ||
+ (READ_ONCE(sk->sk_shutdown) & RCV_SHUTDOWN) ||
signal_pending(current) ||
!timeo)
break;
@@ -2764,7 +2764,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
err = sock_error(sk);
if (err)
goto unlock;
- if (sk->sk_shutdown & RCV_SHUTDOWN)
+ if (READ_ONCE(sk->sk_shutdown) & RCV_SHUTDOWN)
goto unlock;
unix_state_unlock(sk);
--
2.43.0
Powered by blists - more mailing lists