netdev - Re: [fixed] [patch] Re: [bug] stuck localhost TCP connections, v2.6.26-rc3+

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1212537241.6980.12.camel@localhost>
Date:	Tue, 03 Jun 2008 16:54:01 -0700
From:	Joe Perches <joe@...ches.com>
To:	Ilpo Järvinen <ilpo.jarvinen@...sinki.fi>
Cc:	David Miller <davem@...emloft.net>, mingo@...e.hu,
	mcmanus@...ksong.com, peterz@...radead.org,
	LKML <linux-kernel@...r.kernel.org>,
	Netdev <netdev@...r.kernel.org>, rjw@...k.pl,
	Andrew Morton <akpm@...ux-foundation.org>, johnpol@....mipt.ru
Subject: Re: [fixed] [patch] Re: [bug] stuck localhost TCP connections,
	v2.6.26-rc3+

On Wed, 2008-06-04 at 02:22 +0300, Ilpo Järvinen wrote:
> But here's somewhat more likely explanation... Only compile tested...
> It probably needs some commenting from people who understand locking 
> variants & details (I don't).
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...sinki.fi>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index c9454f0..d21d2b9 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -4562,6 +4562,7 @@ static int tcp_defer_accept_check(struct sock *sk)
>  	struct tcp_sock *tp = tcp_sk(sk);
>  
>  	if (tp->defer_tcp_accept.request) {
> +		struct sock *listen_sk = tp->defer_tcp_accept.listen_sk;

Not commenting on the locking, but I think the code
would be clearer if tp->defer_tcp_accept was used in
a temporary instead.

Perhaps:

 net/ipv4/tcp_input.c |   33 ++++++++++++++++-----------------
 1 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index b54d9d3..f846e11 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4535,18 +4535,18 @@ static void tcp_urg(struct sock *sk, struct sk_buff *skb, struct tcphdr *th)
 static int tcp_defer_accept_check(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
-
-	if (tp->defer_tcp_accept.request) {
-		int queued_data =  tp->rcv_nxt - tp->copied_seq;
-		int hasfin =  !skb_queue_empty(&sk->sk_receive_queue) ?
+	struct tcp_deferred_accept_info *tdai = &tp->defer_tcp_accept;
+	if (tdai->request) {
+		int queued_data = tp->rcv_nxt - tp->copied_seq;
+		int hasfin = !skb_queue_empty(&sk->sk_receive_queue) ?
 			tcp_hdr((struct sk_buff *)
 				sk->sk_receive_queue.prev)->fin : 0;
 
 		if (queued_data && hasfin)
 			queued_data--;
 
-		if (queued_data &&
-		    tp->defer_tcp_accept.listen_sk->sk_state == TCP_LISTEN) {
+		bh_lock_sock(tdai->listen_sk);
+		if (queued_data && tdai->listen_sk->sk_state == TCP_LISTEN) {
 			if (sock_flag(sk, SOCK_KEEPOPEN)) {
 				inet_csk_reset_keepalive_timer(sk,
 							       keepalive_time_when(tp));
@@ -4554,23 +4554,22 @@ static int tcp_defer_accept_check(struct sock *sk)
 				inet_csk_delete_keepalive_timer(sk);
 			}
 
-			inet_csk_reqsk_queue_add(
-				tp->defer_tcp_accept.listen_sk,
-				tp->defer_tcp_accept.request,
-				sk);
+			inet_csk_reqsk_queue_add(tdai->listen_sk,
+						 tdai->request,
+						 sk);
 
-			tp->defer_tcp_accept.listen_sk->sk_data_ready(
-				tp->defer_tcp_accept.listen_sk, 0);
+			tdai->listen_sk->sk_data_ready(tdai->listen_sk, 0);
 
-			sock_put(tp->defer_tcp_accept.listen_sk);
+			sock_put(tdai->listen_sk);
 			sock_put(sk);
-			tp->defer_tcp_accept.listen_sk = NULL;
-			tp->defer_tcp_accept.request = NULL;
-		} else if (hasfin ||
-			   tp->defer_tcp_accept.listen_sk->sk_state != TCP_LISTEN) {
+			tdai->listen_sk = NULL;
+			tdai->request = NULL;
+		} else if (hasfin || tdai->listen_sk->sk_state != TCP_LISTEN) {
+			bh_unlock_sock(tdai->listen_sk);
 			tcp_reset(sk);
 			return -1;
 		}
+		bh_unlock_sock(tdai->listen_sk);
 	}
 	return 0;
 }


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html