Commit cfac7f83 authored by Eric Dumazet's avatar Eric Dumazet Committed by David S. Miller

tcp/dccp: block bh before arming time_wait timer

Maciej Żenczykowski reported some panics in tcp_twsk_destructor()
that might be caused by the following bug.

timewait timer is pinned to the cpu, because we want to transition
timwewait refcount from 0 to 4 in one go, once everything has been
initialized.

At the time commit ed2e9239 ("tcp/dccp: fix timewait races in timer
handling") was merged, TCP was always running from BH habdler.

After commit 5413d1ba ("net: do not block BH while processing
socket backlog") we definitely can run tcp_time_wait() from process
context.

We need to block BH in the critical section so that the pinned timer
has still its purpose.

This bug is more likely to happen under stress and when very small RTO
are used in datacenter flows.

Fixes: 5413d1ba ("net: do not block BH while processing socket backlog")
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Reported-by: default avatarMaciej Żenczykowski <maze@google.com>
Acked-by: default avatarMaciej Żenczykowski <maze@google.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent b484d8a5
...@@ -57,10 +57,16 @@ void dccp_time_wait(struct sock *sk, int state, int timeo) ...@@ -57,10 +57,16 @@ void dccp_time_wait(struct sock *sk, int state, int timeo)
if (state == DCCP_TIME_WAIT) if (state == DCCP_TIME_WAIT)
timeo = DCCP_TIMEWAIT_LEN; timeo = DCCP_TIMEWAIT_LEN;
/* tw_timer is pinned, so we need to make sure BH are disabled
* in following section, otherwise timer handler could run before
* we complete the initialization.
*/
local_bh_disable();
inet_twsk_schedule(tw, timeo); inet_twsk_schedule(tw, timeo);
/* Linkage updates. */ /* Linkage updates. */
__inet_twsk_hashdance(tw, sk, &dccp_hashinfo); __inet_twsk_hashdance(tw, sk, &dccp_hashinfo);
inet_twsk_put(tw); inet_twsk_put(tw);
local_bh_enable();
} else { } else {
/* Sorry, if we're out of memory, just CLOSE this /* Sorry, if we're out of memory, just CLOSE this
* socket up. We've got bigger problems than * socket up. We've got bigger problems than
......
...@@ -310,10 +310,16 @@ void tcp_time_wait(struct sock *sk, int state, int timeo) ...@@ -310,10 +310,16 @@ void tcp_time_wait(struct sock *sk, int state, int timeo)
if (state == TCP_TIME_WAIT) if (state == TCP_TIME_WAIT)
timeo = TCP_TIMEWAIT_LEN; timeo = TCP_TIMEWAIT_LEN;
/* tw_timer is pinned, so we need to make sure BH are disabled
* in following section, otherwise timer handler could run before
* we complete the initialization.
*/
local_bh_disable();
inet_twsk_schedule(tw, timeo); inet_twsk_schedule(tw, timeo);
/* Linkage updates. */ /* Linkage updates. */
__inet_twsk_hashdance(tw, sk, &tcp_hashinfo); __inet_twsk_hashdance(tw, sk, &tcp_hashinfo);
inet_twsk_put(tw); inet_twsk_put(tw);
local_bh_enable();
} else { } else {
/* Sorry, if we're out of memory, just CLOSE this /* Sorry, if we're out of memory, just CLOSE this
* socket up. We've got bigger problems than * socket up. We've got bigger problems than
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment