All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ahelenia Ziemiańska" <nabijaczleweli@nabijaczleweli.xyz>
Cc: Jens Axboe <axboe@kernel.dk>,
	Christian Brauner <brauner@kernel.org>,
	 Alexander Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org,
	 Eric Dumazet <edumazet@google.com>,
	"David S. Miller" <davem@davemloft.net>,
	 David Ahern <dsahern@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>,
	 Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v2 07/11] net/tcp: tcp_splice_read: always do non-blocking reads
Date: Thu, 21 Dec 2023 04:09:08 +0100	[thread overview]
Message-ID: <d2d856ed990f713d03e72c72dc81097467cbf983.1703126594.git.nabijaczleweli@nabijaczleweli.xyz> (raw)
In-Reply-To: <cover.1703126594.git.nabijaczleweli@nabijaczleweli.xyz>

[-- Attachment #1: Type: text/plain, Size: 2846 bytes --]

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@nabijaczleweli.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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  parent reply	other threads:[~2023-12-21  3:09 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-21  3:08 [PATCH v2 00/11] Avoid unprivileged splice(file->)/(->socket) pipe exclusion Ahelenia Ziemiańska
2023-12-21  3:08 ` [PATCH v2 01/11] splice: copy_splice_read: do the I/O with IOCB_NOWAIT Ahelenia Ziemiańska
2023-12-21  8:27   ` Christoph Hellwig
2023-12-21 16:30     ` Ahelenia Ziemiańska
2023-12-21  3:08 ` [PATCH v2 02/11] af_unix: unix_stream_splice_read: always request MSG_DONTWAIT Ahelenia Ziemiańska
2023-12-21  3:08 ` [PATCH v2 03/11] fuse: fuse_dev_splice_read: use nonblocking I/O Ahelenia Ziemiańska
2023-12-21  3:09 ` [PATCH v2 04/11] net/smc: smc_splice_read: always request MSG_DONTWAIT Ahelenia Ziemiańska
2023-12-21  3:09 ` [PATCH v2 05/11] kcm: kcm_splice_read: " Ahelenia Ziemiańska
2023-12-21  3:09 ` [PATCH v2 06/11] tls/sw: tls_sw_splice_read: always request non-blocking I/O Ahelenia Ziemiańska
2023-12-21  3:09 ` Ahelenia Ziemiańska [this message]
2023-12-21  3:09 ` [PATCH v2 08/11] tty: splice_read: disable Ahelenia Ziemiańska
2023-12-21  8:10   ` Greg Kroah-Hartman
2024-01-03 11:36   ` Jiri Slaby
2024-01-03 19:14     ` Linus Torvalds
2024-01-03 21:34       ` Oliver Giles
2024-01-03 21:57         ` Linus Torvalds
2023-12-21  3:09 ` [PATCH v2 09/11] fuse: file: limit splice_read to virtiofs Ahelenia Ziemiańska
2024-01-10 13:43   ` Miklos Szeredi
2024-01-10 15:19     ` Ahelenia Ziemiańska
2024-01-10 15:47       ` Miklos Szeredi
2023-12-21  3:09 ` [PATCH v2 10/11] fuse: allow splicing from filesystems mounted by real root Ahelenia Ziemiańska
2023-12-21  3:09 ` [PATCH v2 11/11] splice: splice_to_socket: always request MSG_DONTWAIT Ahelenia Ziemiańska
2023-12-21  3:09 ` [PATCH v2 12/11 man-pages] splice.2: document 6.8 blocking behaviour Ahelenia Ziemiańska
2023-12-24  5:01 ` [PATCH v2 13/11] tty: splice_write: disable Ahelenia Ziemiańska
2023-12-24  5:01 ` [PATCH v2 14/11] fuse: allow splicing to trusted mounts only Ahelenia Ziemiańska

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d2d856ed990f713d03e72c72dc81097467cbf983.1703126594.git.nabijaczleweli@nabijaczleweli.xyz \
    --to=nabijaczleweli@nabijaczleweli.xyz \
    --cc=axboe@kernel.dk \
    --cc=brauner@kernel.org \
    --cc=davem@davemloft.net \
    --cc=dsahern@kernel.org \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.