netdev - Re: [PATCH net v2] af_unix: Fix data races in unix_release_sock/unix_stream

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240510044948.26074-1-kuniyu@amazon.com>
Date: Fri, 10 May 2024 13:49:48 +0900
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <leitao@...ian.org>
CC: <alexander@...alicyn.com>, <daan.j.demeyer@...il.com>,
	<davem@...emloft.net>, <dhowells@...hat.com>, <edumazet@...gle.com>,
	<horms@...nel.org>, <john.fastabend@...il.com>, <kuba@...nel.org>,
	<kuniyu@...zon.com>, <linux-kernel@...r.kernel.org>,
	<netdev@...r.kernel.org>, <pabeni@...hat.com>, <paulmck@...nel.org>
Subject: Re: [PATCH net v2] af_unix: Fix data races in unix_release_sock/unix_stream_sendmsg

From: Breno Leitao <leitao@...ian.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@...ian.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@...zon.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
>