* [PATCH v1 net 0/4] af_unix: Fix four data-races.
@ 2023-09-02 0:27 Kuniyuki Iwashima
2023-09-02 0:27 ` [PATCH v1 net 1/4] af_unix: Fix data-races around user->unix_inflight Kuniyuki Iwashima
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2023-09-02 0:27 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Kuniyuki Iwashima, Kuniyuki Iwashima, netdev
While running syzkaller, KCSAN reported 3 data-races with
systemd-coredump using AF_UNIX sockets.
This series fixes the three and another one inspiered by
one of the reports.
Kuniyuki Iwashima (4):
af_unix: Fix data-races around user->unix_inflight.
af_unix: Fix data-race around unix_tot_inflight.
af_unix: Fix data-races around sk->sk_shutdown.
af_unix: Fix data race around sk->sk_err.
net/core/sock.c | 6 +++---
net/unix/af_unix.c | 2 +-
net/unix/scm.c | 6 +++---
3 files changed, 7 insertions(+), 7 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v1 net 1/4] af_unix: Fix data-races around user->unix_inflight.
2023-09-02 0:27 [PATCH v1 net 0/4] af_unix: Fix four data-races Kuniyuki Iwashima
@ 2023-09-02 0:27 ` Kuniyuki Iwashima
2023-09-02 5:45 ` Willy Tarreau
2023-09-02 6:39 ` Eric Dumazet
2023-09-02 0:27 ` [PATCH v1 net 2/4] af_unix: Fix data-race around unix_tot_inflight Kuniyuki Iwashima
` (3 subsequent siblings)
4 siblings, 2 replies; 11+ messages in thread
From: Kuniyuki Iwashima @ 2023-09-02 0:27 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Kuniyuki Iwashima, Kuniyuki Iwashima, netdev, syzkaller,
Willy Tarreau
user->unix_inflight is changed under spin_lock(unix_gc_lock),
but too_many_unix_fds() reads it locklessly.
Let's annotate the write/read accesses to user->unix_inflight.
BUG: KCSAN: data-race in unix_attach_fds / unix_inflight
write to 0xffffffff8546f2d0 of 8 bytes by task 44798 on cpu 1:
unix_inflight+0x157/0x180 net/unix/scm.c:66
unix_attach_fds+0x147/0x1e0 net/unix/scm.c:123
unix_scm_to_skb net/unix/af_unix.c:1827 [inline]
unix_dgram_sendmsg+0x46a/0x14f0 net/unix/af_unix.c:1950
unix_seqpacket_sendmsg net/unix/af_unix.c:2308 [inline]
unix_seqpacket_sendmsg+0xba/0x130 net/unix/af_unix.c:2292
sock_sendmsg_nosec net/socket.c:725 [inline]
sock_sendmsg+0x148/0x160 net/socket.c:748
____sys_sendmsg+0x4e4/0x610 net/socket.c:2494
___sys_sendmsg+0xc6/0x140 net/socket.c:2548
__sys_sendmsg+0x94/0x140 net/socket.c:2577
__do_sys_sendmsg net/socket.c:2586 [inline]
__se_sys_sendmsg net/socket.c:2584 [inline]
__x64_sys_sendmsg+0x45/0x50 net/socket.c:2584
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x3b/0x90 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x6e/0xd8
read to 0xffffffff8546f2d0 of 8 bytes by task 44814 on cpu 0:
too_many_unix_fds net/unix/scm.c:101 [inline]
unix_attach_fds+0x54/0x1e0 net/unix/scm.c:110
unix_scm_to_skb net/unix/af_unix.c:1827 [inline]
unix_dgram_sendmsg+0x46a/0x14f0 net/unix/af_unix.c:1950
unix_seqpacket_sendmsg net/unix/af_unix.c:2308 [inline]
unix_seqpacket_sendmsg+0xba/0x130 net/unix/af_unix.c:2292
sock_sendmsg_nosec net/socket.c:725 [inline]
sock_sendmsg+0x148/0x160 net/socket.c:748
____sys_sendmsg+0x4e4/0x610 net/socket.c:2494
___sys_sendmsg+0xc6/0x140 net/socket.c:2548
__sys_sendmsg+0x94/0x140 net/socket.c:2577
__do_sys_sendmsg net/socket.c:2586 [inline]
__se_sys_sendmsg net/socket.c:2584 [inline]
__x64_sys_sendmsg+0x45/0x50 net/socket.c:2584
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x3b/0x90 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x6e/0xd8
value changed: 0x000000000000000c -> 0x000000000000000d
Reported by Kernel Concurrency Sanitizer on:
CPU: 0 PID: 44814 Comm: systemd-coredum Not tainted 6.4.0-11989-g6843306689af #6
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
Fixes: 712f4aad406b ("unix: properly account for FDs passed over unix sockets")
Reported-by: syzkaller <syzkaller@googlegroups.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
Cc: Willy Tarreau <w@1wt.eu>
---
net/unix/scm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/unix/scm.c b/net/unix/scm.c
index e9dde7176c8a..6ff628f2349f 100644
--- a/net/unix/scm.c
+++ b/net/unix/scm.c
@@ -64,7 +64,7 @@ void unix_inflight(struct user_struct *user, struct file *fp)
/* Paired with READ_ONCE() in wait_for_unix_gc() */
WRITE_ONCE(unix_tot_inflight, unix_tot_inflight + 1);
}
- user->unix_inflight++;
+ WRITE_ONCE(user->unix_inflight, user->unix_inflight + 1);
spin_unlock(&unix_gc_lock);
}
@@ -85,7 +85,7 @@ void unix_notinflight(struct user_struct *user, struct file *fp)
/* Paired with READ_ONCE() in wait_for_unix_gc() */
WRITE_ONCE(unix_tot_inflight, unix_tot_inflight - 1);
}
- user->unix_inflight--;
+ WRITE_ONCE(user->unix_inflight, user->unix_inflight - 1);
spin_unlock(&unix_gc_lock);
}
@@ -99,7 +99,7 @@ static inline bool too_many_unix_fds(struct task_struct *p)
{
struct user_struct *user = current_user();
- if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
+ if (unlikely(READ_ONCE(user->unix_inflight) > task_rlimit(p, RLIMIT_NOFILE)))
return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
return false;
}
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 net 2/4] af_unix: Fix data-race around unix_tot_inflight.
2023-09-02 0:27 [PATCH v1 net 0/4] af_unix: Fix four data-races Kuniyuki Iwashima
2023-09-02 0:27 ` [PATCH v1 net 1/4] af_unix: Fix data-races around user->unix_inflight Kuniyuki Iwashima
@ 2023-09-02 0:27 ` Kuniyuki Iwashima
2023-09-02 6:41 ` Eric Dumazet
2023-09-02 0:27 ` [PATCH v1 net 3/4] af_unix: Fix data-races around sk->sk_shutdown Kuniyuki Iwashima
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Kuniyuki Iwashima @ 2023-09-02 0:27 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Kuniyuki Iwashima, Kuniyuki Iwashima, netdev, syzkaller,
Pavel Emelyanov
unix_tot_inflight is changed under spin_lock(unix_gc_lock), but
unix_release_sock() reads it locklessly.
Let's use READ_ONCE() for unix_tot_inflight.
Note that the writer side was marked by commit 9d6d7f1cb67c ("af_unix:
annote lockless accesses to unix_tot_inflight & gc_in_progress")
BUG: KCSAN: data-race in unix_inflight / unix_release_sock
write (marked) to 0xffffffff871852b8 of 4 bytes by task 123 on cpu 1:
unix_inflight+0x130/0x180 net/unix/scm.c:64
unix_attach_fds+0x137/0x1b0 net/unix/scm.c:123
unix_scm_to_skb net/unix/af_unix.c:1832 [inline]
unix_dgram_sendmsg+0x46a/0x14f0 net/unix/af_unix.c:1955
sock_sendmsg_nosec net/socket.c:724 [inline]
sock_sendmsg+0x148/0x160 net/socket.c:747
____sys_sendmsg+0x4e4/0x610 net/socket.c:2493
___sys_sendmsg+0xc6/0x140 net/socket.c:2547
__sys_sendmsg+0x94/0x140 net/socket.c:2576
__do_sys_sendmsg net/socket.c:2585 [inline]
__se_sys_sendmsg net/socket.c:2583 [inline]
__x64_sys_sendmsg+0x45/0x50 net/socket.c:2583
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x3b/0x90 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x72/0xdc
read to 0xffffffff871852b8 of 4 bytes by task 4891 on cpu 0:
unix_release_sock+0x608/0x910 net/unix/af_unix.c:671
unix_release+0x59/0x80 net/unix/af_unix.c:1058
__sock_release+0x7d/0x170 net/socket.c:653
sock_close+0x19/0x30 net/socket.c:1385
__fput+0x179/0x5e0 fs/file_table.c:321
____fput+0x15/0x20 fs/file_table.c:349
task_work_run+0x116/0x1a0 kernel/task_work.c:179
resume_user_mode_work include/linux/resume_user_mode.h:49 [inline]
exit_to_user_mode_loop kernel/entry/common.c:171 [inline]
exit_to_user_mode_prepare+0x174/0x180 kernel/entry/common.c:204
__syscall_exit_to_user_mode_work kernel/entry/common.c:286 [inline]
syscall_exit_to_user_mode+0x1a/0x30 kernel/entry/common.c:297
do_syscall_64+0x4b/0x90 arch/x86/entry/common.c:86
entry_SYSCALL_64_after_hwframe+0x72/0xdc
value changed: 0x00000000 -> 0x00000001
Reported by Kernel Concurrency Sanitizer on:
CPU: 0 PID: 4891 Comm: systemd-coredum Not tainted 6.4.0-rc5-01219-gfa0e21fa4443 #5
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
Fixes: 9305cfa4443d ("[AF_UNIX]: Make unix_tot_inflight counter non-atomic")
Reported-by: syzkaller <syzkaller@googlegroups.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
Cc: Pavel Emelyanov <xemul@openvz.org>
---
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 86930a8ed012..3e8a04a13668 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -680,7 +680,7 @@ static void unix_release_sock(struct sock *sk, int embrion)
* What the above comment does talk about? --ANK(980817)
*/
- if (unix_tot_inflight)
+ if (READ_ONCE(unix_tot_inflight))
unix_gc(); /* Garbage collect fds */
}
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 net 3/4] af_unix: Fix data-races around sk->sk_shutdown.
2023-09-02 0:27 [PATCH v1 net 0/4] af_unix: Fix four data-races Kuniyuki Iwashima
2023-09-02 0:27 ` [PATCH v1 net 1/4] af_unix: Fix data-races around user->unix_inflight Kuniyuki Iwashima
2023-09-02 0:27 ` [PATCH v1 net 2/4] af_unix: Fix data-race around unix_tot_inflight Kuniyuki Iwashima
@ 2023-09-02 0:27 ` Kuniyuki Iwashima
2023-09-02 6:43 ` Eric Dumazet
2023-09-02 0:27 ` [PATCH v1 net 4/4] af_unix: Fix data race around sk->sk_err Kuniyuki Iwashima
2023-09-04 10:12 ` [PATCH v1 net 0/4] af_unix: Fix four data-races patchwork-bot+netdevbpf
4 siblings, 1 reply; 11+ messages in thread
From: Kuniyuki Iwashima @ 2023-09-02 0:27 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Kuniyuki Iwashima, Kuniyuki Iwashima, netdev, syzkaller
sk->sk_shutdown is changed under unix_state_lock(sk), but
unix_dgram_sendmsg() calls two functions to read sk_shutdown locklessly.
sock_alloc_send_pskb
`- sock_wait_for_wmem
Let's use READ_ONCE() there.
Note that the writer side was marked by commit e1d09c2c2f57 ("af_unix:
Fix data races around sk->sk_shutdown.").
BUG: KCSAN: data-race in sock_alloc_send_pskb / unix_release_sock
write (marked) to 0xffff8880069af12c of 1 bytes by task 1 on cpu 1:
unix_release_sock+0x75c/0x910 net/unix/af_unix.c:631
unix_release+0x59/0x80 net/unix/af_unix.c:1053
__sock_release+0x7d/0x170 net/socket.c:654
sock_close+0x19/0x30 net/socket.c:1386
__fput+0x2a3/0x680 fs/file_table.c:384
____fput+0x15/0x20 fs/file_table.c:412
task_work_run+0x116/0x1a0 kernel/task_work.c:179
resume_user_mode_work include/linux/resume_user_mode.h:49 [inline]
exit_to_user_mode_loop kernel/entry/common.c:171 [inline]
exit_to_user_mode_prepare+0x174/0x180 kernel/entry/common.c:204
__syscall_exit_to_user_mode_work kernel/entry/common.c:286 [inline]
syscall_exit_to_user_mode+0x1a/0x30 kernel/entry/common.c:297
do_syscall_64+0x4b/0x90 arch/x86/entry/common.c:86
entry_SYSCALL_64_after_hwframe+0x6e/0xd8
read to 0xffff8880069af12c of 1 bytes by task 28650 on cpu 0:
sock_alloc_send_pskb+0xd2/0x620 net/core/sock.c:2767
unix_dgram_sendmsg+0x2f8/0x14f0 net/unix/af_unix.c:1944
unix_seqpacket_sendmsg net/unix/af_unix.c:2308 [inline]
unix_seqpacket_sendmsg+0xba/0x130 net/unix/af_unix.c:2292
sock_sendmsg_nosec net/socket.c:725 [inline]
sock_sendmsg+0x148/0x160 net/socket.c:748
____sys_sendmsg+0x4e4/0x610 net/socket.c:2494
___sys_sendmsg+0xc6/0x140 net/socket.c:2548
__sys_sendmsg+0x94/0x140 net/socket.c:2577
__do_sys_sendmsg net/socket.c:2586 [inline]
__se_sys_sendmsg net/socket.c:2584 [inline]
__x64_sys_sendmsg+0x45/0x50 net/socket.c:2584
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x3b/0x90 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x6e/0xd8
value changed: 0x00 -> 0x03
Reported by Kernel Concurrency Sanitizer on:
CPU: 0 PID: 28650 Comm: systemd-coredum Not tainted 6.4.0-11989-g6843306689af #6
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>
---
net/core/sock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/sock.c b/net/core/sock.c
index d3c7b53368d2..e3da7eae9338 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2747,7 +2747,7 @@ static long sock_wait_for_wmem(struct sock *sk, long timeo)
prepare_to_wait(sk_sleep(sk), &wait, TASK_INTERRUPTIBLE);
if (refcount_read(&sk->sk_wmem_alloc) < READ_ONCE(sk->sk_sndbuf))
break;
- if (sk->sk_shutdown & SEND_SHUTDOWN)
+ if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN)
break;
if (sk->sk_err)
break;
@@ -2777,7 +2777,7 @@ struct sk_buff *sock_alloc_send_pskb(struct sock *sk, unsigned long header_len,
goto failure;
err = -EPIPE;
- if (sk->sk_shutdown & SEND_SHUTDOWN)
+ if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN)
goto failure;
if (sk_wmem_alloc_get(sk) < READ_ONCE(sk->sk_sndbuf))
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v1 net 4/4] af_unix: Fix data race around sk->sk_err.
2023-09-02 0:27 [PATCH v1 net 0/4] af_unix: Fix four data-races Kuniyuki Iwashima
` (2 preceding siblings ...)
2023-09-02 0:27 ` [PATCH v1 net 3/4] af_unix: Fix data-races around sk->sk_shutdown Kuniyuki Iwashima
@ 2023-09-02 0:27 ` Kuniyuki Iwashima
2023-09-02 6:45 ` Eric Dumazet
2023-09-04 10:12 ` [PATCH v1 net 0/4] af_unix: Fix four data-races patchwork-bot+netdevbpf
4 siblings, 1 reply; 11+ messages in thread
From: Kuniyuki Iwashima @ 2023-09-02 0:27 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Kuniyuki Iwashima, Kuniyuki Iwashima, netdev
As with sk->sk_shutdown shown in the previous patch, sk->sk_err can be
read locklessly by unix_dgram_sendmsg().
Let's use READ_ONCE() for sk_err as well.
Note that the writer side is marked by commit cc04410af7de ("af_unix:
annotate lockless accesses to sk->sk_err").
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
net/core/sock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/sock.c b/net/core/sock.c
index e3da7eae9338..16584e2dd648 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2749,7 +2749,7 @@ static long sock_wait_for_wmem(struct sock *sk, long timeo)
break;
if (READ_ONCE(sk->sk_shutdown) & SEND_SHUTDOWN)
break;
- if (sk->sk_err)
+ if (READ_ONCE(sk->sk_err))
break;
timeo = schedule_timeout(timeo);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v1 net 1/4] af_unix: Fix data-races around user->unix_inflight.
2023-09-02 0:27 ` [PATCH v1 net 1/4] af_unix: Fix data-races around user->unix_inflight Kuniyuki Iwashima
@ 2023-09-02 5:45 ` Willy Tarreau
2023-09-02 6:39 ` Eric Dumazet
1 sibling, 0 replies; 11+ messages in thread
From: Willy Tarreau @ 2023-09-02 5:45 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Kuniyuki Iwashima, netdev, syzkaller
Hi,
On Fri, Sep 01, 2023 at 05:27:05PM -0700, Kuniyuki Iwashima wrote:
> user->unix_inflight is changed under spin_lock(unix_gc_lock),
> but too_many_unix_fds() reads it locklessly.
>
> Let's annotate the write/read accesses to user->unix_inflight.
>
> BUG: KCSAN: data-race in unix_attach_fds / unix_inflight
>
> write to 0xffffffff8546f2d0 of 8 bytes by task 44798 on cpu 1:
> unix_inflight+0x157/0x180 net/unix/scm.c:66
> unix_attach_fds+0x147/0x1e0 net/unix/scm.c:123
> unix_scm_to_skb net/unix/af_unix.c:1827 [inline]
> unix_dgram_sendmsg+0x46a/0x14f0 net/unix/af_unix.c:1950
> unix_seqpacket_sendmsg net/unix/af_unix.c:2308 [inline]
> unix_seqpacket_sendmsg+0xba/0x130 net/unix/af_unix.c:2292
> sock_sendmsg_nosec net/socket.c:725 [inline]
> sock_sendmsg+0x148/0x160 net/socket.c:748
> ____sys_sendmsg+0x4e4/0x610 net/socket.c:2494
> ___sys_sendmsg+0xc6/0x140 net/socket.c:2548
> __sys_sendmsg+0x94/0x140 net/socket.c:2577
> __do_sys_sendmsg net/socket.c:2586 [inline]
> __se_sys_sendmsg net/socket.c:2584 [inline]
> __x64_sys_sendmsg+0x45/0x50 net/socket.c:2584
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x3b/0x90 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x6e/0xd8
>
> read to 0xffffffff8546f2d0 of 8 bytes by task 44814 on cpu 0:
> too_many_unix_fds net/unix/scm.c:101 [inline]
> unix_attach_fds+0x54/0x1e0 net/unix/scm.c:110
> unix_scm_to_skb net/unix/af_unix.c:1827 [inline]
> unix_dgram_sendmsg+0x46a/0x14f0 net/unix/af_unix.c:1950
> unix_seqpacket_sendmsg net/unix/af_unix.c:2308 [inline]
> unix_seqpacket_sendmsg+0xba/0x130 net/unix/af_unix.c:2292
> sock_sendmsg_nosec net/socket.c:725 [inline]
> sock_sendmsg+0x148/0x160 net/socket.c:748
> ____sys_sendmsg+0x4e4/0x610 net/socket.c:2494
> ___sys_sendmsg+0xc6/0x140 net/socket.c:2548
> __sys_sendmsg+0x94/0x140 net/socket.c:2577
> __do_sys_sendmsg net/socket.c:2586 [inline]
> __se_sys_sendmsg net/socket.c:2584 [inline]
> __x64_sys_sendmsg+0x45/0x50 net/socket.c:2584
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x3b/0x90 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x6e/0xd8
>
> value changed: 0x000000000000000c -> 0x000000000000000d
>
> Reported by Kernel Concurrency Sanitizer on:
> CPU: 0 PID: 44814 Comm: systemd-coredum Not tainted 6.4.0-11989-g6843306689af #6
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
>
> Fixes: 712f4aad406b ("unix: properly account for FDs passed over unix sockets")
> Reported-by: syzkaller <syzkaller@googlegroups.com>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
> Cc: Willy Tarreau <w@1wt.eu>
> ---
> net/unix/scm.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/unix/scm.c b/net/unix/scm.c
> index e9dde7176c8a..6ff628f2349f 100644
> --- a/net/unix/scm.c
> +++ b/net/unix/scm.c
> @@ -64,7 +64,7 @@ void unix_inflight(struct user_struct *user, struct file *fp)
> /* Paired with READ_ONCE() in wait_for_unix_gc() */
> WRITE_ONCE(unix_tot_inflight, unix_tot_inflight + 1);
> }
> - user->unix_inflight++;
> + WRITE_ONCE(user->unix_inflight, user->unix_inflight + 1);
> spin_unlock(&unix_gc_lock);
> }
>
> @@ -85,7 +85,7 @@ void unix_notinflight(struct user_struct *user, struct file *fp)
> /* Paired with READ_ONCE() in wait_for_unix_gc() */
> WRITE_ONCE(unix_tot_inflight, unix_tot_inflight - 1);
> }
> - user->unix_inflight--;
> + WRITE_ONCE(user->unix_inflight, user->unix_inflight - 1);
> spin_unlock(&unix_gc_lock);
> }
>
> @@ -99,7 +99,7 @@ static inline bool too_many_unix_fds(struct task_struct *p)
> {
> struct user_struct *user = current_user();
>
> - if (unlikely(user->unix_inflight > task_rlimit(p, RLIMIT_NOFILE)))
> + if (unlikely(READ_ONCE(user->unix_inflight) > task_rlimit(p, RLIMIT_NOFILE)))
> return !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN);
> return false;
> }
Looks good to me, thanks!
Acked-by: Willy Tarreau <w@1wt.eu>
Willy
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 net 1/4] af_unix: Fix data-races around user->unix_inflight.
2023-09-02 0:27 ` [PATCH v1 net 1/4] af_unix: Fix data-races around user->unix_inflight Kuniyuki Iwashima
2023-09-02 5:45 ` Willy Tarreau
@ 2023-09-02 6:39 ` Eric Dumazet
1 sibling, 0 replies; 11+ messages in thread
From: Eric Dumazet @ 2023-09-02 6:39 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima,
netdev, syzkaller, Willy Tarreau
On Sat, Sep 2, 2023 at 2:28 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> user->unix_inflight is changed under spin_lock(unix_gc_lock),
> but too_many_unix_fds() reads it locklessly.
>
> Let's annotate the write/read accesses to user->unix_inflight.
>
> BUG: KCSAN: data-race in unix_attach_fds / unix_inflight
>
> write to 0xffffffff8546f2d0 of 8 bytes by task 44798 on cpu 1:
> unix_inflight+0x157/0x180 net/unix/scm.c:66
> unix_attach_fds+0x147/0x1e0 net/unix/scm.c:123
> unix_scm_to_skb net/unix/af_unix.c:1827 [inline]
> unix_dgram_sendmsg+0x46a/0x14f0 net/unix/af_unix.c:1950
> unix_seqpacket_sendmsg net/unix/af_unix.c:2308 [inline]
> unix_seqpacket_sendmsg+0xba/0x130 net/unix/af_unix.c:2292
> sock_sendmsg_nosec net/socket.c:725 [inline]
> sock_sendmsg+0x148/0x160 net/socket.c:748
> ____sys_sendmsg+0x4e4/0x610 net/socket.c:2494
> ___sys_sendmsg+0xc6/0x140 net/socket.c:2548
> __sys_sendmsg+0x94/0x140 net/socket.c:2577
> __do_sys_sendmsg net/socket.c:2586 [inline]
> __se_sys_sendmsg net/socket.c:2584 [inline]
> __x64_sys_sendmsg+0x45/0x50 net/socket.c:2584
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x3b/0x90 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x6e/0xd8
>
> read to 0xffffffff8546f2d0 of 8 bytes by task 44814 on cpu 0:
> too_many_unix_fds net/unix/scm.c:101 [inline]
> unix_attach_fds+0x54/0x1e0 net/unix/scm.c:110
> unix_scm_to_skb net/unix/af_unix.c:1827 [inline]
> unix_dgram_sendmsg+0x46a/0x14f0 net/unix/af_unix.c:1950
> unix_seqpacket_sendmsg net/unix/af_unix.c:2308 [inline]
> unix_seqpacket_sendmsg+0xba/0x130 net/unix/af_unix.c:2292
> sock_sendmsg_nosec net/socket.c:725 [inline]
> sock_sendmsg+0x148/0x160 net/socket.c:748
> ____sys_sendmsg+0x4e4/0x610 net/socket.c:2494
> ___sys_sendmsg+0xc6/0x140 net/socket.c:2548
> __sys_sendmsg+0x94/0x140 net/socket.c:2577
> __do_sys_sendmsg net/socket.c:2586 [inline]
> __se_sys_sendmsg net/socket.c:2584 [inline]
> __x64_sys_sendmsg+0x45/0x50 net/socket.c:2584
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x3b/0x90 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x6e/0xd8
>
> value changed: 0x000000000000000c -> 0x000000000000000d
>
> Reported by Kernel Concurrency Sanitizer on:
> CPU: 0 PID: 44814 Comm: systemd-coredum Not tainted 6.4.0-11989-g6843306689af #6
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
>
> Fixes: 712f4aad406b ("unix: properly account for FDs passed over unix sockets")
> Reported-by: syzkaller <syzkaller@googlegroups.com>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 net 2/4] af_unix: Fix data-race around unix_tot_inflight.
2023-09-02 0:27 ` [PATCH v1 net 2/4] af_unix: Fix data-race around unix_tot_inflight Kuniyuki Iwashima
@ 2023-09-02 6:41 ` Eric Dumazet
0 siblings, 0 replies; 11+ messages in thread
From: Eric Dumazet @ 2023-09-02 6:41 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima,
netdev, syzkaller, Pavel Emelyanov
On Sat, Sep 2, 2023 at 2:28 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> unix_tot_inflight is changed under spin_lock(unix_gc_lock), but
> unix_release_sock() reads it locklessly.
>
> Let's use READ_ONCE() for unix_tot_inflight.
>
> Note that the writer side was marked by commit 9d6d7f1cb67c ("af_unix:
> annote lockless accesses to unix_tot_inflight & gc_in_progress")
>
>
> Fixes: 9305cfa4443d ("[AF_UNIX]: Make unix_tot_inflight counter non-atomic")
> Reported-by: syzkaller <syzkaller@googlegroups.com>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 net 3/4] af_unix: Fix data-races around sk->sk_shutdown.
2023-09-02 0:27 ` [PATCH v1 net 3/4] af_unix: Fix data-races around sk->sk_shutdown Kuniyuki Iwashima
@ 2023-09-02 6:43 ` Eric Dumazet
0 siblings, 0 replies; 11+ messages in thread
From: Eric Dumazet @ 2023-09-02 6:43 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima,
netdev, syzkaller
On Sat, Sep 2, 2023 at 2:28 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> sk->sk_shutdown is changed under unix_state_lock(sk), but
> unix_dgram_sendmsg() calls two functions to read sk_shutdown locklessly.
>
> sock_alloc_send_pskb
> `- sock_wait_for_wmem
>
> Let's use READ_ONCE() there.
>
> Note that the writer side was marked by commit e1d09c2c2f57 ("af_unix:
> Fix data races around sk->sk_shutdown.").
>
> BUG: KCSAN: data-race in sock_alloc_send_pskb / unix_release_sock
>
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: syzkaller <syzkaller@googlegroups.com>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 net 4/4] af_unix: Fix data race around sk->sk_err.
2023-09-02 0:27 ` [PATCH v1 net 4/4] af_unix: Fix data race around sk->sk_err Kuniyuki Iwashima
@ 2023-09-02 6:45 ` Eric Dumazet
0 siblings, 0 replies; 11+ messages in thread
From: Eric Dumazet @ 2023-09-02 6:45 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Kuniyuki Iwashima,
netdev
On Sat, Sep 2, 2023 at 2:29 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> As with sk->sk_shutdown shown in the previous patch, sk->sk_err can be
> read locklessly by unix_dgram_sendmsg().
>
> Let's use READ_ONCE() for sk_err as well.
>
> Note that the writer side is marked by commit cc04410af7de ("af_unix:
> annotate lockless accesses to sk->sk_err").
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
Title is slightly off, since this is a generic net: change, but this
is fine really ;)
Reviewed-by: Eric Dumazet <edumazet@google.com>
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v1 net 0/4] af_unix: Fix four data-races.
2023-09-02 0:27 [PATCH v1 net 0/4] af_unix: Fix four data-races Kuniyuki Iwashima
` (3 preceding siblings ...)
2023-09-02 0:27 ` [PATCH v1 net 4/4] af_unix: Fix data race around sk->sk_err Kuniyuki Iwashima
@ 2023-09-04 10:12 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-09-04 10:12 UTC (permalink / raw)
To: Kuniyuki Iwashima; +Cc: davem, edumazet, kuba, pabeni, kuni1840, netdev
Hello:
This series was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:
On Fri, 1 Sep 2023 17:27:04 -0700 you wrote:
> While running syzkaller, KCSAN reported 3 data-races with
> systemd-coredump using AF_UNIX sockets.
>
> This series fixes the three and another one inspiered by
> one of the reports.
>
>
> [...]
Here is the summary with links:
- [v1,net,1/4] af_unix: Fix data-races around user->unix_inflight.
https://git.kernel.org/netdev/net/c/0bc36c0650b2
- [v1,net,2/4] af_unix: Fix data-race around unix_tot_inflight.
https://git.kernel.org/netdev/net/c/ade32bd8a738
- [v1,net,3/4] af_unix: Fix data-races around sk->sk_shutdown.
https://git.kernel.org/netdev/net/c/afe8764f7634
- [v1,net,4/4] af_unix: Fix data race around sk->sk_err.
https://git.kernel.org/netdev/net/c/b192812905e4
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-09-04 10:12 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-02 0:27 [PATCH v1 net 0/4] af_unix: Fix four data-races Kuniyuki Iwashima
2023-09-02 0:27 ` [PATCH v1 net 1/4] af_unix: Fix data-races around user->unix_inflight Kuniyuki Iwashima
2023-09-02 5:45 ` Willy Tarreau
2023-09-02 6:39 ` Eric Dumazet
2023-09-02 0:27 ` [PATCH v1 net 2/4] af_unix: Fix data-race around unix_tot_inflight Kuniyuki Iwashima
2023-09-02 6:41 ` Eric Dumazet
2023-09-02 0:27 ` [PATCH v1 net 3/4] af_unix: Fix data-races around sk->sk_shutdown Kuniyuki Iwashima
2023-09-02 6:43 ` Eric Dumazet
2023-09-02 0:27 ` [PATCH v1 net 4/4] af_unix: Fix data race around sk->sk_err Kuniyuki Iwashima
2023-09-02 6:45 ` Eric Dumazet
2023-09-04 10:12 ` [PATCH v1 net 0/4] af_unix: Fix four data-races patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).