netdev - [PATCH v2 07/11] net/tcp: tcp_splice_read: always do non-blocking reads

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] [day] [month] [year] [list]

Date: Thu, 21 Dec 2023 04:09:08 +0100
From: 
	Ahelenia Ziemiańska <nabijaczleweli@...ijaczleweli.xyz>
Cc: Jens Axboe <axboe@...nel.dk>, Christian Brauner <brauner@...nel.org>, 
	Alexander Viro <viro@...iv.linux.org.uk>, linux-fsdevel@...r.kernel.org, 
	Eric Dumazet <edumazet@...gle.com>, "David S. Miller" <davem@...emloft.net>, 
	David Ahern <dsahern@...nel.org>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH v2 07/11] net/tcp: tcp_splice_read: always do non-blocking
 reads

Otherwise we risk sleeping with the pipe locked for indeterminate
lengths of time ‒ given:
	cat > tcp.c <<^D
	#define _GNU_SOURCE
	#include <fcntl.h>
	#include <unistd.h>
	#include <sys/socket.h>
	#include <netinet/in.h>
	#include <linux/tls.h>
	int main()
	{
		int s = socket(AF_INET, SOCK_STREAM, 0);
		struct sockaddr_in addr = {
			.sin_family = AF_INET,
			.sin_addr = { htonl(INADDR_LOOPBACK) },
			.sin_port = htons(getpid() % (0xFFFF - 1000) + 1000)
		};
		bind(s, &addr, sizeof(addr));
		listen(s, 1);
		if (!fork()) {
			connect(socket(AF_INET, SOCK_STREAM, 0), &addr, sizeof(addr));
			sleep(100);
			return 0;
		}

		s = accept(s, NULL, NULL);
		for (;;)
			splice(s, 0, 1, 0, 128 * 1024 * 1024, 0);
	}
	^D
	cc tcp.c -o tcp
	mkfifo fifo
	./tcp > fifo &
	read -r _ < fifo &
	sleep 0.1
	echo zupa > fifo
tcp used to sleep in splice and the shell used to enter an
uninterruptible sleep in open("fifo");
now the splice returns -EAGAIN and the whole program completes.

sock_rcvtimeo() returns 0 if the second argument is true, so the
explicit re-try loop for empty read conditions can be removed
entirely.

Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@...ijaczleweli.xyz>
---
 net/ipv4/tcp.c | 32 +++-----------------------------
 1 file changed, 3 insertions(+), 29 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index ff6838ca2e58..17a0e2a766b7 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -782,7 +782,6 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
 		.len = len,
 		.flags = flags,
 	};
-	long timeo;
 	ssize_t spliced;
 	int ret;
 
@@ -797,7 +796,6 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
 
 	lock_sock(sk);
 
-	timeo = sock_rcvtimeo(sk, sock->file->f_flags & O_NONBLOCK);
 	while (tss.len) {
 		ret = __tcp_splice_read(sk, &tss);
 		if (ret < 0)
@@ -821,37 +819,13 @@ ssize_t tcp_splice_read(struct socket *sock, loff_t *ppos,
 				ret = -ENOTCONN;
 				break;
 			}
-			if (!timeo) {
-				ret = -EAGAIN;
-				break;
-			}
-			/* if __tcp_splice_read() got nothing while we have
-			 * an skb in receive queue, we do not want to loop.
-			 * This might happen with URG data.
-			 */
-			if (!skb_queue_empty(&sk->sk_receive_queue))
-				break;
-			ret = sk_wait_data(sk, &timeo, NULL);
-			if (ret < 0)
-				break;
-			if (signal_pending(current)) {
-				ret = sock_intr_errno(timeo);
-				break;
-			}
-			continue;
+			ret = -EAGAIN;
+			break;
 		}
 		tss.len -= ret;
 		spliced += ret;
 
-		if (!tss.len || !timeo)
-			break;
-		release_sock(sk);
-		lock_sock(sk);
-
-		if (sk->sk_err || sk->sk_state == TCP_CLOSE ||
-		    (sk->sk_shutdown & RCV_SHUTDOWN) ||
-		    signal_pending(current))
-			break;
+		break;
 	}
 
 	release_sock(sk);
-- 
2.39.2

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)