Skip to content

Commit 878d951

Browse files
edumazetPaolo Abeni
authored andcommitted
inet: lock the socket in ip_sock_set_tos()
Christoph Paasch reported a panic in TCP stack [1] Indeed, we should not call sk_dst_reset() without holding the socket lock, as __sk_dst_get() callers do not all rely on bare RCU. [1] BUG: kernel NULL pointer dereference, address: 0000000000000000 PGD 12bad6067 P4D 12bad6067 PUD 12bad5067 PMD 0 Oops: 0000 [#1] PREEMPT SMP CPU: 1 PID: 2750 Comm: syz-executor.5 Not tainted 6.6.0-rc4-g7a5720a344e7 #49 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.el7 04/01/2014 RIP: 0010:tcp_get_metrics+0x118/0x8f0 net/ipv4/tcp_metrics.c:321 Code: c7 44 24 70 02 00 8b 03 89 44 24 48 c7 44 24 4c 00 00 00 00 66 c7 44 24 58 02 00 66 ba 02 00 b1 01 89 4c 24 04 4c 89 7c 24 10 <49> 8b 0f 48 8b 89 50 05 00 00 48 89 4c 24 30 33 81 00 02 00 00 69 RSP: 0018:ffffc90000af79b8 EFLAGS: 00010293 RAX: 000000000100007f RBX: ffff88812ae8f500 RCX: ffff88812b5f8f01 RDX: 0000000000000002 RSI: ffffffff8300f080 RDI: 0000000000000002 RBP: 0000000000000002 R08: 0000000000000003 R09: ffffffff8205eca0 R10: 0000000000000002 R11: ffff88812b5f8f00 R12: ffff88812a9e0580 R13: 0000000000000000 R14: ffff88812ae8fbd2 R15: 0000000000000000 FS: 00007f70a006b640(0000) GS:ffff88813bd00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 000000012bad7003 CR4: 0000000000170ee0 Call Trace: <TASK> tcp_fastopen_cache_get+0x32/0x140 net/ipv4/tcp_metrics.c:567 tcp_fastopen_cookie_check+0x28/0x180 net/ipv4/tcp_fastopen.c:419 tcp_connect+0x9c8/0x12a0 net/ipv4/tcp_output.c:3839 tcp_v4_connect+0x645/0x6e0 net/ipv4/tcp_ipv4.c:323 __inet_stream_connect+0x120/0x590 net/ipv4/af_inet.c:676 tcp_sendmsg_fastopen+0x2d6/0x3a0 net/ipv4/tcp.c:1021 tcp_sendmsg_locked+0x1957/0x1b00 net/ipv4/tcp.c:1073 tcp_sendmsg+0x30/0x50 net/ipv4/tcp.c:1336 __sock_sendmsg+0x83/0xd0 net/socket.c:730 __sys_sendto+0x20a/0x2a0 net/socket.c:2194 __do_sys_sendto net/socket.c:2206 [inline] Fixes: e08d0b3 ("inet: implement lockless IP_TOS") Reported-by: Christoph Paasch <[email protected]> Signed-off-by: Eric Dumazet <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Paolo Abeni <[email protected]>
1 parent 70b9a3d commit 878d951

File tree

4 files changed

+13
-5
lines changed

4 files changed

+13
-5
lines changed

include/net/ip.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,5 +810,6 @@ int ip_sock_set_mtu_discover(struct sock *sk, int val);
810810
void ip_sock_set_pktinfo(struct sock *sk);
811811
void ip_sock_set_recverr(struct sock *sk);
812812
void ip_sock_set_tos(struct sock *sk, int val);
813+
void __ip_sock_set_tos(struct sock *sk, int val);
813814

814815
#endif /* _IP_H */

net/ipv4/ip_sockglue.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -585,9 +585,9 @@ int ip_recv_error(struct sock *sk, struct msghdr *msg, int len, int *addr_len)
585585
return err;
586586
}
587587

588-
void ip_sock_set_tos(struct sock *sk, int val)
588+
void __ip_sock_set_tos(struct sock *sk, int val)
589589
{
590-
u8 old_tos = READ_ONCE(inet_sk(sk)->tos);
590+
u8 old_tos = inet_sk(sk)->tos;
591591

592592
if (sk->sk_type == SOCK_STREAM) {
593593
val &= ~INET_ECN_MASK;
@@ -599,6 +599,13 @@ void ip_sock_set_tos(struct sock *sk, int val)
599599
sk_dst_reset(sk);
600600
}
601601
}
602+
603+
void ip_sock_set_tos(struct sock *sk, int val)
604+
{
605+
lock_sock(sk);
606+
__ip_sock_set_tos(sk, val);
607+
release_sock(sk);
608+
}
602609
EXPORT_SYMBOL(ip_sock_set_tos);
603610

604611
void ip_sock_set_freebind(struct sock *sk)

net/mptcp/sockopt.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -738,7 +738,7 @@ static int mptcp_setsockopt_v4_set_tos(struct mptcp_sock *msk, int optname,
738738
mptcp_for_each_subflow(msk, subflow) {
739739
struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
740740

741-
ip_sock_set_tos(ssk, val);
741+
__ip_sock_set_tos(ssk, val);
742742
}
743743
release_sock(sk);
744744

@@ -1411,7 +1411,7 @@ static void sync_socket_options(struct mptcp_sock *msk, struct sock *ssk)
14111411
ssk->sk_bound_dev_if = sk->sk_bound_dev_if;
14121412
ssk->sk_incoming_cpu = sk->sk_incoming_cpu;
14131413
ssk->sk_ipv6only = sk->sk_ipv6only;
1414-
ip_sock_set_tos(ssk, inet_sk(sk)->tos);
1414+
__ip_sock_set_tos(ssk, inet_sk(sk)->tos);
14151415

14161416
if (sk->sk_userlocks & tx_rx_locks) {
14171417
ssk->sk_userlocks |= sk->sk_userlocks & tx_rx_locks;

tools/testing/selftests/net/mptcp/mptcp_connect.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -716,7 +716,7 @@ run_test_transparent()
716716
# the required infrastructure in MPTCP sockopt code. To support TOS, the
717717
# following function has been exported (T). Not great but better than
718718
# checking for a specific kernel version.
719-
if ! mptcp_lib_kallsyms_has "T ip_sock_set_tos$"; then
719+
if ! mptcp_lib_kallsyms_has "T __ip_sock_set_tos$"; then
720720
echo "INFO: ${msg} not supported by the kernel: SKIP"
721721
mptcp_lib_result_skip "${TEST_GROUP}"
722722
return

0 commit comments

Comments
 (0)