[PATCH net] tcp: tcp_rtx_synack() can be called from process context

[PATCH net] tcp: tcp_rtx_synack() can be called from process context

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Laurent reported the enclosed report [1]

This bug triggers with following coditions:

0) Kernel built with CONFIG_DEBUG_PREEMPT=y

1) A new passive FastOpen TCP socket is created.
   This FO socket waits for an ACK coming from client to be a complete
   ESTABLISHED one.
2) A socket operation on this socket goes through lock_sock()
   release_sock() dance.
3) While the socket is owned by the user in step 2),
   a retransmit of the SYN is received and stored in socket backlog.
4) At release_sock() time, the socket backlog is processed while
   in process context.
5) A SYNACK packet is cooked in response of the SYN retransmit.
6) -> tcp_rtx_synack() is called in process context.

Before blamed commit, tcp_rtx_synack() was always called from BH handler,
from a timer handler.

Fix this by using TCP_INC_STATS() & NET_INC_STATS()
which do not assume caller is in non preemptible context.

[1]
BUG: using __this_cpu_add() in preemptible [00000000] code: epollpep/2180
caller is tcp_rtx_synack.part.0+0x36/0xc0
CPU: 10 PID: 2180 Comm: epollpep Tainted: G           OE     5.16.0-0.bpo.4-amd64 #1  Debian 5.16.12-1~bpo11+1
Hardware name: Supermicro SYS-5039MC-H8TRF/X11SCD-F, BIOS 1.7 11/23/2021
Call Trace:
 <TASK>
 dump_stack_lvl+0x48/0x5e
 check_preemption_disabled+0xde/0xe0
 tcp_rtx_synack.part.0+0x36/0xc0
 tcp_rtx_synack+0x8d/0xa0
 ? kmem_cache_alloc+0x2e0/0x3e0
 ? apparmor_file_alloc_security+0x3b/0x1f0
 inet_rtx_syn_ack+0x16/0x30
 tcp_check_req+0x367/0x610
 tcp_rcv_state_process+0x91/0xf60
 ? get_nohz_timer_target+0x18/0x1a0
 ? lock_timer_base+0x61/0x80
 ? preempt_count_add+0x68/0xa0
 tcp_v4_do_rcv+0xbd/0x270
 __release_sock+0x6d/0xb0
 release_sock+0x2b/0x90
 sock_setsockopt+0x138/0x1140
 ? __sys_getsockname+0x7e/0xc0
 ? aa_sk_perm+0x3e/0x1a0
 __sys_setsockopt+0x198/0x1e0
 __x64_sys_setsockopt+0x21/0x30
 do_syscall_64+0x38/0xc0
 entry_SYSCALL_64_after_hwframe+0x44/0xae

Fixes: 168a8f58059a ("tcp: TCP Fast Open Server - main code path")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Laurent Fasnacht <laurent.fasnacht@proton.ch>
---
 net/ipv4/tcp_output.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index b4b2284ed4a2c9e2569bd945e3b4e023c5502f25..1c054431e358328fe3849f5a45aaa88308a1e1c8 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -4115,8 +4115,8 @@ int tcp_rtx_synack(const struct sock *sk, struct request_sock *req)
 	res = af_ops->send_synack(sk, NULL, &fl, req, NULL, TCP_SYNACK_NORMAL,
 				  NULL);
 	if (!res) {
-		__TCP_INC_STATS(sock_net(sk), TCP_MIB_RETRANSSEGS);
-		__NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPSYNRETRANS);
+		TCP_INC_STATS(sock_net(sk), TCP_MIB_RETRANSSEGS);
+		NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPSYNRETRANS);
 		if (unlikely(tcp_passive_fastopen(sk)))
 			tcp_sk(sk)->total_retrans++;
 		trace_tcp_retransmit_synack(sk, req);
-- 
2.36.1.255.ge46751e96f-goog

Re: [PATCH net] tcp: tcp_rtx_synack() can be called from process context

[Neal Cardwell]

On Mon, May 30, 2022 at 5:37 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> From: Eric Dumazet <edumazet@google.com>
>
> Laurent reported the enclosed report [1]
>
> This bug triggers with following coditions:
>
> 0) Kernel built with CONFIG_DEBUG_PREEMPT=y
>
> 1) A new passive FastOpen TCP socket is created.
>    This FO socket waits for an ACK coming from client to be a complete
>    ESTABLISHED one.
> 2) A socket operation on this socket goes through lock_sock()
>    release_sock() dance.
> 3) While the socket is owned by the user in step 2),
>    a retransmit of the SYN is received and stored in socket backlog.
> 4) At release_sock() time, the socket backlog is processed while
>    in process context.
> 5) A SYNACK packet is cooked in response of the SYN retransmit.
> 6) -> tcp_rtx_synack() is called in process context.
>
> Before blamed commit, tcp_rtx_synack() was always called from BH handler,
> from a timer handler.
>
> Fix this by using TCP_INC_STATS() & NET_INC_STATS()
> which do not assume caller is in non preemptible context.
>
> [1]
> BUG: using __this_cpu_add() in preemptible [00000000] code: epollpep/2180
> caller is tcp_rtx_synack.part.0+0x36/0xc0
> CPU: 10 PID: 2180 Comm: epollpep Tainted: G           OE     5.16.0-0.bpo.4-amd64 #1  Debian 5.16.12-1~bpo11+1
> Hardware name: Supermicro SYS-5039MC-H8TRF/X11SCD-F, BIOS 1.7 11/23/2021
> Call Trace:
>  <TASK>
>  dump_stack_lvl+0x48/0x5e
>  check_preemption_disabled+0xde/0xe0
>  tcp_rtx_synack.part.0+0x36/0xc0
>  tcp_rtx_synack+0x8d/0xa0
>  ? kmem_cache_alloc+0x2e0/0x3e0
>  ? apparmor_file_alloc_security+0x3b/0x1f0
>  inet_rtx_syn_ack+0x16/0x30
>  tcp_check_req+0x367/0x610
>  tcp_rcv_state_process+0x91/0xf60
>  ? get_nohz_timer_target+0x18/0x1a0
>  ? lock_timer_base+0x61/0x80
>  ? preempt_count_add+0x68/0xa0
>  tcp_v4_do_rcv+0xbd/0x270
>  __release_sock+0x6d/0xb0
>  release_sock+0x2b/0x90
>  sock_setsockopt+0x138/0x1140
>  ? __sys_getsockname+0x7e/0xc0
>  ? aa_sk_perm+0x3e/0x1a0
>  __sys_setsockopt+0x198/0x1e0
>  __x64_sys_setsockopt+0x21/0x30
>  do_syscall_64+0x38/0xc0
>  entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> Fixes: 168a8f58059a ("tcp: TCP Fast Open Server - main code path")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Laurent Fasnacht <laurent.fasnacht@proton.ch>
> ---
>  net/ipv4/tcp_output.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index b4b2284ed4a2c9e2569bd945e3b4e023c5502f25..1c054431e358328fe3849f5a45aaa88308a1e1c8 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -4115,8 +4115,8 @@ int tcp_rtx_synack(const struct sock *sk, struct request_sock *req)
>         res = af_ops->send_synack(sk, NULL, &fl, req, NULL, TCP_SYNACK_NORMAL,
>                                   NULL);
>         if (!res) {
> -               __TCP_INC_STATS(sock_net(sk), TCP_MIB_RETRANSSEGS);
> -               __NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPSYNRETRANS);
> +               TCP_INC_STATS(sock_net(sk), TCP_MIB_RETRANSSEGS);
> +               NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPSYNRETRANS);
>                 if (unlikely(tcp_passive_fastopen(sk)))
>                         tcp_sk(sk)->total_retrans++;
>                 trace_tcp_retransmit_synack(sk, req);
> --

Nice diagnosis and fix! Thanks, Eric!

Acked-by: Neal Cardwell <ncardwell@google.com>

neal

Re: [PATCH net] tcp: tcp_rtx_synack() can be called from process context

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 30 May 2022 14:37:13 -0700 you wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> Laurent reported the enclosed report [1]
> 
> This bug triggers with following coditions:
> 
> 0) Kernel built with CONFIG_DEBUG_PREEMPT=y
> 
> [...]

Here is the summary with links:
  - [net] tcp: tcp_rtx_synack() can be called from process context
    https://git.kernel.org/netdev/net/c/0a375c822497

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html