Skip to content

Commit e14cadf

Browse files
edumazetdavem330
authored andcommitted
tcp: add annotations around sk->sk_shutdown accesses
Now sk->sk_shutdown is no longer a bitfield, we can add standard READ_ONCE()/WRITE_ONCE() annotations to silence KCSAN reports like the following: BUG: KCSAN: data-race in tcp_disconnect / tcp_poll write to 0xffff88814588582c of 1 bytes by task 3404 on cpu 1: tcp_disconnect+0x4d6/0xdb0 net/ipv4/tcp.c:3121 __inet_stream_connect+0x5dd/0x6e0 net/ipv4/af_inet.c:715 inet_stream_connect+0x48/0x70 net/ipv4/af_inet.c:727 __sys_connect_file net/socket.c:2001 [inline] __sys_connect+0x19b/0x1b0 net/socket.c:2018 __do_sys_connect net/socket.c:2028 [inline] __se_sys_connect net/socket.c:2025 [inline] __x64_sys_connect+0x41/0x50 net/socket.c:2025 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x63/0xcd read to 0xffff88814588582c of 1 bytes by task 3374 on cpu 0: tcp_poll+0x2e6/0x7d0 net/ipv4/tcp.c:562 sock_poll+0x253/0x270 net/socket.c:1383 vfs_poll include/linux/poll.h:88 [inline] io_poll_check_events io_uring/poll.c:281 [inline] io_poll_task_func+0x15a/0x820 io_uring/poll.c:333 handle_tw_list io_uring/io_uring.c:1184 [inline] tctx_task_work+0x1fe/0x4d0 io_uring/io_uring.c:1246 task_work_run+0x123/0x160 kernel/task_work.c:179 get_signal+0xe64/0xff0 kernel/signal.c:2635 arch_do_signal_or_restart+0x89/0x2a0 arch/x86/kernel/signal.c:306 exit_to_user_mode_loop+0x6f/0xe0 kernel/entry/common.c:168 exit_to_user_mode_prepare+0x6c/0xb0 kernel/entry/common.c:204 __syscall_exit_to_user_mode_work kernel/entry/common.c:286 [inline] syscall_exit_to_user_mode+0x26/0x140 kernel/entry/common.c:297 do_syscall_64+0x4d/0xc0 arch/x86/entry/common.c:86 entry_SYSCALL_64_after_hwframe+0x63/0xcd value changed: 0x03 -> 0x00 Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Reported-by: syzbot <syzkaller@googlegroups.com> Signed-off-by: Eric Dumazet <edumazet@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 4063384 commit e14cadf

3 files changed

Lines changed: 11 additions & 9 deletions

File tree

net/ipv4/af_inet.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -894,7 +894,7 @@ int inet_shutdown(struct socket *sock, int how)
894894
EPOLLHUP, even on eg. unconnected UDP sockets -- RR */
895895
fallthrough;
896896
default:
897-
sk->sk_shutdown |= how;
897+
WRITE_ONCE(sk->sk_shutdown, sk->sk_shutdown | how);
898898
if (sk->sk_prot->shutdown)
899899
sk->sk_prot->shutdown(sk, how);
900900
break;

net/ipv4/tcp.c

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,7 @@ __poll_t tcp_poll(struct file *file, struct socket *sock, poll_table *wait)
498498
__poll_t mask;
499499
struct sock *sk = sock->sk;
500500
const struct tcp_sock *tp = tcp_sk(sk);
501+
u8 shutdown;
501502
int state;
502503

503504
sock_poll_wait(file, sock, wait);
@@ -540,9 +541,10 @@ __poll_t tcp_poll(struct file *file, struct socket *sock, poll_table *wait)
540541
* NOTE. Check for TCP_CLOSE is added. The goal is to prevent
541542
* blocking on fresh not-connected or disconnected socket. --ANK
542543
*/
543-
if (sk->sk_shutdown == SHUTDOWN_MASK || state == TCP_CLOSE)
544+
shutdown = READ_ONCE(sk->sk_shutdown);
545+
if (shutdown == SHUTDOWN_MASK || state == TCP_CLOSE)
544546
mask |= EPOLLHUP;
545-
if (sk->sk_shutdown & RCV_SHUTDOWN)
547+
if (shutdown & RCV_SHUTDOWN)
546548
mask |= EPOLLIN | EPOLLRDNORM | EPOLLRDHUP;
547549

548550
/* Connected or passive Fast Open socket? */
@@ -559,7 +561,7 @@ __poll_t tcp_poll(struct file *file, struct socket *sock, poll_table *wait)
559561
if (tcp_stream_is_readable(sk, target))
560562
mask |= EPOLLIN | EPOLLRDNORM;
561563

562-
if (!(sk->sk_shutdown & SEND_SHUTDOWN)) {
564+
if (!(shutdown & SEND_SHUTDOWN)) {
563565
if (__sk_stream_is_writeable(sk, 1)) {
564566
mask |= EPOLLOUT | EPOLLWRNORM;
565567
} else { /* send SIGIO later */
@@ -2867,7 +2869,7 @@ void __tcp_close(struct sock *sk, long timeout)
28672869
int data_was_unread = 0;
28682870
int state;
28692871

2870-
sk->sk_shutdown = SHUTDOWN_MASK;
2872+
WRITE_ONCE(sk->sk_shutdown, SHUTDOWN_MASK);
28712873

28722874
if (sk->sk_state == TCP_LISTEN) {
28732875
tcp_set_state(sk, TCP_CLOSE);
@@ -3119,7 +3121,7 @@ int tcp_disconnect(struct sock *sk, int flags)
31193121

31203122
inet_bhash2_reset_saddr(sk);
31213123

3122-
sk->sk_shutdown = 0;
3124+
WRITE_ONCE(sk->sk_shutdown, 0);
31233125
sock_reset_flag(sk, SOCK_DONE);
31243126
tp->srtt_us = 0;
31253127
tp->mdev_us = jiffies_to_usecs(TCP_TIMEOUT_INIT);
@@ -4649,7 +4651,7 @@ void tcp_done(struct sock *sk)
46494651
if (req)
46504652
reqsk_fastopen_remove(sk, req, false);
46514653

4652-
sk->sk_shutdown = SHUTDOWN_MASK;
4654+
WRITE_ONCE(sk->sk_shutdown, SHUTDOWN_MASK);
46534655

46544656
if (!sock_flag(sk, SOCK_DEAD))
46554657
sk->sk_state_change(sk);

net/ipv4/tcp_input.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4362,7 +4362,7 @@ void tcp_fin(struct sock *sk)
43624362

43634363
inet_csk_schedule_ack(sk);
43644364

4365-
sk->sk_shutdown |= RCV_SHUTDOWN;
4365+
WRITE_ONCE(sk->sk_shutdown, sk->sk_shutdown | RCV_SHUTDOWN);
43664366
sock_set_flag(sk, SOCK_DONE);
43674367

43684368
switch (sk->sk_state) {
@@ -6599,7 +6599,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
65996599
break;
66006600

66016601
tcp_set_state(sk, TCP_FIN_WAIT2);
6602-
sk->sk_shutdown |= SEND_SHUTDOWN;
6602+
WRITE_ONCE(sk->sk_shutdown, sk->sk_shutdown | SEND_SHUTDOWN);
66036603

66046604
sk_dst_confirm(sk);
66056605

0 commit comments

Comments
 (0)