af_unix: Annotate data-race around unix_sk(sk)->addr.

Once unix_sk(sk)->addr is assigned under net->unx.table.locks and
unix_sk(sk)->bindlock, *(unix_sk(sk)->addr) and unix_sk(sk)->path are
fully set up, and unix_sk(sk)->addr is never changed.

unix_getname() and unix_copy_addr() access the two fields locklessly,
and commit ae3b564179bf ("missing barriers in some of unix_sock ->addr
and ->path accesses") added smp_store_release() and smp_load_acquire()
pairs.

In other functions, we still read unix_sk(sk)->addr locklessly to check
if the socket is bound, and KCSAN complains about it.  [0]

Given these functions have no dependency for *(unix_sk(sk)->addr) and
unix_sk(sk)->path, READ_ONCE() is enough to annotate the data-race.

Note that it is safe to access unix_sk(sk)->addr locklessly if the socket
is found in the hash table.  For example, the lockless read of otheru->addr
in unix_stream_connect() is safe.

Note also that newu->addr there is of the child socket that is still not
accessible from userspace, and smp_store_release() publishes the address
in case the socket is accept()ed and unix_getname() / unix_copy_addr()
is called.

[0]:
BUG: KCSAN: data-race in unix_bind / unix_listen

write (marked) to 0xffff88805f8d1840 of 8 bytes by task 13723 on cpu 0:
 __unix_set_addr_hash net/unix/af_unix.c:329 [inline]
 unix_bind_bsd net/unix/af_unix.c:1241 [inline]
 unix_bind+0x881/0x1000 net/unix/af_unix.c:1319
 __sys_bind+0x194/0x1e0 net/socket.c:1847
 __do_sys_bind net/socket.c:1858 [inline]
 __se_sys_bind net/socket.c:1856 [inline]
 __x64_sys_bind+0x40/0x50 net/socket.c:1856
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0x4f/0x110 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x46/0x4e

read to 0xffff88805f8d1840 of 8 bytes by task 13724 on cpu 1:
 unix_listen+0x72/0x180 net/unix/af_unix.c:734
 __sys_listen+0xdc/0x160 net/socket.c:1881
 __do_sys_listen net/socket.c:1890 [inline]
 __se_sys_listen net/socket.c:1888 [inline]
 __x64_sys_listen+0x2e/0x40 net/socket.c:1888
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0x4f/0x110 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x46/0x4e

value changed: 0x0000000000000000 -> 0xffff88807b5b1b40

Reported by Kernel Concurrency Sanitizer on:
CPU: 1 PID: 13724 Comm: syz-executor.4 Not tainted 6.8.0-12822-gcd51db110a7e #12
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzkaller <syzkaller@googlegroups.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Link: https://lore.kernel.org/r/20240522154002.77857-1-kuniyu@amazon.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index e4af6616e1dff1da32772eddfdf645fb4c9fa45c..fe631212a345a285f26aa39a5d7770c277f7bac8 100644 (file)
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -731,7 +731,7 @@ static int unix_listen(struct socket *sock, int backlog)
        if (sock->type != SOCK_STREAM && sock->type != SOCK_SEQPACKET)
                goto out;       /* Only stream/seqpacket sockets accept */
        err = -EINVAL;
-       if (!u->addr)
+       if (!READ_ONCE(u->addr))
                goto out;       /* No listens on an unbound socket */
        unix_state_lock(sk);
        if (sk->sk_state != TCP_CLOSE && sk->sk_state != TCP_LISTEN)
@@ -1369,7 +1369,7 @@ static int unix_dgram_connect(struct socket *sock, struct sockaddr *addr,
 
                if ((test_bit(SOCK_PASSCRED, &sock->flags) ||
                     test_bit(SOCK_PASSPIDFD, &sock->flags)) &&
-                   !unix_sk(sk)->addr) {
+                   !READ_ONCE(unix_sk(sk)->addr)) {
                        err = unix_autobind(sk);
                        if (err)
                                goto out;
@@ -1481,7 +1481,8 @@ static int unix_stream_connect(struct socket *sock, struct sockaddr *uaddr,
                goto out;
 
        if ((test_bit(SOCK_PASSCRED, &sock->flags) ||
-            test_bit(SOCK_PASSPIDFD, &sock->flags)) && !u->addr) {
+            test_bit(SOCK_PASSPIDFD, &sock->flags)) &&
+           !READ_ONCE(u->addr)) {
                err = unix_autobind(sk);
                if (err)
                        goto out;
@@ -1950,7 +1951,8 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
        }
 
        if ((test_bit(SOCK_PASSCRED, &sock->flags) ||
-            test_bit(SOCK_PASSPIDFD, &sock->flags)) && !u->addr) {
+            test_bit(SOCK_PASSPIDFD, &sock->flags)) &&
+           !READ_ONCE(u->addr)) {
                err = unix_autobind(sk);
                if (err)
                        goto out;