[PATC net-next] tcp: must block bh in __inet_twsk_hashdance()

[PATC net-next] tcp: must block bh in __inet_twsk_hashdance()

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

__inet_twsk_hashdance() might be called from process context,
better block BH before acquiring bind hash and established locks

Fixes: c10d9310edf5 ("tcp: do not assume TCP code is non preemptible")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/inet_timewait_sock.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
index 99ee5c4a9b6844929a995b0b4b5bd693bb211123..2065816748066986f0356df168c2d76fe2d53d85 100644
--- a/net/ipv4/inet_timewait_sock.c
+++ b/net/ipv4/inet_timewait_sock.c
@@ -94,7 +94,7 @@ static void inet_twsk_add_bind_node(struct inet_timewait_sock *tw,
 }
 
 /*
- * Enter the time wait state. This is called with locally disabled BH.
+ * Enter the time wait state.
  * Essentially we whip up a timewait bucket, copy the relevant info into it
  * from the SK, and mess with hash chains and list linkage.
  */
@@ -112,7 +112,7 @@ void __inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
 	 */
 	bhead = &hashinfo->bhash[inet_bhashfn(twsk_net(tw), inet->inet_num,
 			hashinfo->bhash_size)];
-	spin_lock(&bhead->lock);
+	spin_lock_bh(&bhead->lock);
 	tw->tw_tb = icsk->icsk_bind_hash;
 	WARN_ON(!icsk->icsk_bind_hash);
 	inet_twsk_add_bind_node(tw, &tw->tw_tb->owners);
@@ -138,7 +138,7 @@ void __inet_twsk_hashdance(struct inet_timewait_sock *tw, struct sock *sk,
 	if (__sk_nulls_del_node_init_rcu(sk))
 		sock_prot_inuse_add(sock_net(sk), sk->sk_prot, -1);
 
-	spin_unlock(lock);
+	spin_unlock_bh(lock);
 }
 EXPORT_SYMBOL_GPL(__inet_twsk_hashdance);
 

Re: [PATC net-next] tcp: must block bh in __inet_twsk_hashdance()

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 03 May 2016 17:10:50 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> __inet_twsk_hashdance() might be called from process context,
> better block BH before acquiring bind hash and established locks
> 
> Fixes: c10d9310edf5 ("tcp: do not assume TCP code is non preemptible")
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.

Re: [PATC net-next] tcp: must block bh in __inet_twsk_hashdance()

[Eric Dumazet]

On Wed, 2016-05-04 at 00:54 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 03 May 2016 17:10:50 -0700
> 
> > From: Eric Dumazet <edumazet@google.com>
> > 
> > __inet_twsk_hashdance() might be called from process context,
> > better block BH before acquiring bind hash and established locks
> > 
> > Fixes: c10d9310edf5 ("tcp: do not assume TCP code is non preemptible")
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> 
> Applied.

David, I do not see these two patches in net-next ?

(This one, and the "tcp: fix lockdep splat in tcp_snd_una_update()" one)

Thanks

Re: [PATC net-next] tcp: must block bh in __inet_twsk_hashdance()

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 04 May 2016 11:22:11 -0700

> On Wed, 2016-05-04 at 00:54 -0400, David Miller wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Tue, 03 May 2016 17:10:50 -0700
>> 
>> > From: Eric Dumazet <edumazet@google.com>
>> > 
>> > __inet_twsk_hashdance() might be called from process context,
>> > better block BH before acquiring bind hash and established locks
>> > 
>> > Fixes: c10d9310edf5 ("tcp: do not assume TCP code is non preemptible")
>> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>> 
>> Applied.
> 
> David, I do not see these two patches in net-next ?
> 
> (This one, and the "tcp: fix lockdep splat in tcp_snd_una_update()" one)

I pushed them in by hand, they should really be there now.

Thanks for letting me know.

Re: [PATC net-next] tcp: must block bh in __inet_twsk_hashdance()

[Eric Dumazet]

On Wed, 2016-05-04 at 17:06 -0400, David Miller wrote:

> I pushed them in by hand, they should really be there now.
> 
> Thanks for letting me know.

Thanks David

Further tests show two additional missing local_bh_disable() protections
around percpu_counter_dec() and one sk_sockets_allocated_dec()

Will send the patch in few minutes.

[PATCH net-next] tcp: two more missing bh disable

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

percpu_counter only have protection against preemption.

TCP stack uses them possibly from BH, so we need BH protection
in contexts that could be run in process context

Fixes: c10d9310edf5 ("tcp: do not assume TCP code is non preemptible")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/inet_connection_sock.c |    2 ++
 net/ipv4/tcp_ipv4.c             |    2 ++
 2 files changed, 4 insertions(+)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 7ce112aa3a7b50c8c2c97b4e59e86128f90fd7f6..fa8c39804bdbae867dd5c08f1e308202c1aa1c52 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -706,7 +706,9 @@ void inet_csk_destroy_sock(struct sock *sk)
 
 	sk_refcnt_debug_release(sk);
 
+	local_bh_disable();
 	percpu_counter_dec(sk->sk_prot->orphan_count);
+	local_bh_enable();
 	sock_put(sk);
 }
 EXPORT_SYMBOL(inet_csk_destroy_sock);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 761bc492c5e3070212efe91c3940e4712d9d6ef6..a7ab9472d64560d86ea24ac1b6e1a7800f89989d 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1839,7 +1839,9 @@ void tcp_v4_destroy_sock(struct sock *sk)
 	tcp_free_fastopen_req(tp);
 	tcp_saved_syn_free(tp);
 
+	local_bh_disable();
 	sk_sockets_allocated_dec(sk);
+	local_bh_enable();
 
 	if (mem_cgroup_sockets_enabled && sk->sk_memcg)
 		sock_release_memcg(sk);

Re: [PATCH net-next] tcp: two more missing bh disable

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 04 May 2016 15:27:29 -0700

> From: Eric Dumazet <edumazet@google.com>
> 
> percpu_counter only have protection against preemption.
> 
> TCP stack uses them possibly from BH, so we need BH protection
> in contexts that could be run in process context
> 
> Fixes: c10d9310edf5 ("tcp: do not assume TCP code is non preemptible")
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied.