Skip to content

Commit 219d920

Browse files
dhowellskuba-moo
authored andcommitted
splice, net: Fix SPLICE_F_MORE signalling in splice_direct_to_actor()
splice_direct_to_actor() doesn't manage SPLICE_F_MORE correctly[1] - and, as a result, it incorrectly signals/fails to signal MSG_MORE when splicing to a socket. The problem I'm seeing happens when a short splice occurs because we got a short read due to hitting the EOF on a file: as the length read (read_len) is less than the remaining size to be spliced (len), SPLICE_F_MORE (and thus MSG_MORE) is set. The issue is that, for the moment, we have no way to know *why* the short read occurred and so can't make a good decision on whether we *should* keep MSG_MORE set. MSG_SENDPAGE_NOTLAST was added to work around this, but that is also set incorrectly under some circumstances - for example if a short read fills a single pipe_buffer, but the next read would return more (seqfile can do this). This was observed with the multi_chunk_sendfile tests in the tls kselftest program. Some of those tests would hang and time out when the last chunk of file was less than the sendfile request size: build/kselftest/net/tls -r tls.12_aes_gcm.multi_chunk_sendfile This has been observed before[2] and worked around in AF_TLS[3]. Fix this by making splice_direct_to_actor() always signal SPLICE_F_MORE if we haven't yet hit the requested operation size. SPLICE_F_MORE remains signalled if the user passed it in to splice() but otherwise gets cleared when we've read sufficient data to fulfill the request. If, however, we get a premature EOF from ->splice_read(), have sent at least one byte and SPLICE_F_MORE was not set by the caller, ->splice_eof() will be invoked. Signed-off-by: David Howells <[email protected]> cc: Linus Torvalds <[email protected]> cc: Jens Axboe <[email protected]> cc: Christoph Hellwig <[email protected]> cc: Al Viro <[email protected]> cc: Matthew Wilcox <[email protected]> cc: Jan Kara <[email protected]> cc: Jeff Layton <[email protected]> cc: David Hildenbrand <[email protected]> cc: Christian Brauner <[email protected]> cc: Chuck Lever <[email protected]> cc: Boris Pismenny <[email protected]> cc: John Fastabend <[email protected]> cc: [email protected] Link: https://lore.kernel.org/r/[email protected]/ [1] Link: https://lore.kernel.org/r/[email protected]/ [2] Link: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=d452d48b9f8b1a7f8152d33ef52cfd7fe1735b0a [3] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 951ace9 commit 219d920

File tree

1 file changed

+10
-8
lines changed

1 file changed

+10
-8
lines changed

fs/splice.c

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1063,13 +1063,17 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
10631063
*/
10641064
bytes = 0;
10651065
len = sd->total_len;
1066+
1067+
/* Don't block on output, we have to drain the direct pipe. */
10661068
flags = sd->flags;
1069+
sd->flags &= ~SPLICE_F_NONBLOCK;
10671070

10681071
/*
1069-
* Don't block on output, we have to drain the direct pipe.
1072+
* We signal MORE until we've read sufficient data to fulfill the
1073+
* request and we keep signalling it if the caller set it.
10701074
*/
1071-
sd->flags &= ~SPLICE_F_NONBLOCK;
10721075
more = sd->flags & SPLICE_F_MORE;
1076+
sd->flags |= SPLICE_F_MORE;
10731077

10741078
WARN_ON_ONCE(!pipe_empty(pipe->head, pipe->tail));
10751079

@@ -1085,14 +1089,12 @@ ssize_t splice_direct_to_actor(struct file *in, struct splice_desc *sd,
10851089
sd->total_len = read_len;
10861090

10871091
/*
1088-
* If more data is pending, set SPLICE_F_MORE
1089-
* If this is the last data and SPLICE_F_MORE was not set
1090-
* initially, clears it.
1092+
* If we now have sufficient data to fulfill the request then
1093+
* we clear SPLICE_F_MORE if it was not set initially.
10911094
*/
1092-
if (read_len < len)
1093-
sd->flags |= SPLICE_F_MORE;
1094-
else if (!more)
1095+
if (read_len >= len && !more)
10951096
sd->flags &= ~SPLICE_F_MORE;
1097+
10961098
/*
10971099
* NOTE: nonblocking mode only applies to the input. We
10981100
* must not do the output in nonblocking mode as then we

0 commit comments

Comments
 (0)