netdev - Re: strange tcp behavior

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]

Date:	Thu, 2 Aug 2007 22:08:42 +0400
From:	Evgeniy Polyakov <johnpol@....mipt.ru>
To:	Simon Arlott <simon@...e.lp0.eu>
Cc:	john@...een.lv, netdev@...r.kernel.org,
	David Miller <davem@...emloft.net>
Subject: Re: strange tcp behavior

On Thu, Aug 02, 2007 at 06:15:52PM +0100, Simon Arlott (simon@...e.lp0.eu) wrote:
> 17:33:45.351273 IP 192.168.7.4.50000 > 192.168.7.8.2500: R 1385353596:1385353596(0) win 1500
> 17:33:45.360878 IP 192.168.7.8.48186 > 192.168.7.4.50000: R 1388203103:1388203103(0) ack 1385353596 win 14360
> 
> Seems to be losing the source port information when it decides to send 
> that final RST|ACK. It's going through the "TCPAbortOnClose" path:
> 
> tcp_close:
> 	-> tcp_set_state(sk, TCP_CLOSE)
> 		-> inet_put_port(&tcp_hashinfo, sk)
> 		Perhaps it's losing the port information here?
> 	-> tcp_send_active_reset(sk, GFP_KERNEL)
> 
> "TCP_CLOSE       socket is finished"
> Should these two calls be the other way round?
> 
> 
> Also, I don't think it should be sending a RST after the other side has 
> sent one - the connection no longer exists so there is nothing on the 
> other side to reset.

Problem is not in tcp_send_active_reset(), when socket is being released
it is already damaged.
Problem is that inet_autobind() function is called for socket, which is
already dead, but not yet completely - it smells bad (since it has its
port freed), but stil alive (accessible via send()), so for its last
word inet_sendmsg() tries to bind it again, and only after that time it
will be eventually closed and freed completely.

So, following patch fixes problem for me.
Another solution might not to release port until socket is being
released, but that can lead to performance degradation.
Correct me if sk_err can be reset.

Signed-off-by: Evgeniy Polyakov <johnpol@....mipt.ru>

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 06c08e5..6790b23 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -168,8 +169,14 @@ void inet_sock_destruct(struct sock *sk)
 static int inet_autobind(struct sock *sk)
 {
 	struct inet_sock *inet;
+
 	/* We may need to bind the socket. */
 	lock_sock(sk);
+	if (sk->sk_err) {
+		release_sock(sk);
+		return sk->sk_err;
+	}
+
 	inet = inet_sk(sk);
 	if (!inet->num) {
 		if (sk->sk_prot->get_port(sk, 0)) {
@@ -686,8 +703,11 @@ int inet_sendmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
 	struct sock *sk = sock->sk;
 
 	/* We may need to bind the socket. */
-	if (!inet_sk(sk)->num && inet_autobind(sk))
-		return -EAGAIN;
+	if (!inet_sk(sk)->num) {
+		int err = inet_autobind(sk);
+		if (err)
+			return err;
+	}
 
 	return sk->sk_prot->sendmsg(iocb, sk, msg, size);
 }
@@ -698,8 +718,11 @@ static ssize_t inet_sendpage(struct socket *sock, struct page *page, int offset,
 	struct sock *sk = sock->sk;
 
 	/* We may need to bind the socket. */
-	if (!inet_sk(sk)->num && inet_autobind(sk))
-		return -EAGAIN;
+	if (!inet_sk(sk)->num) {
+		int err = inet_autobind(sk);
+		if (err)
+			return err;
+	}
 
 	if (sk->sk_prot->sendpage)
 		return sk->sk_prot->sendpage(sk, page, offset, size, flags);

-- 
	Evgeniy Polyakov
-
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