From: Jakub Kicinski <kuba@kernel.org>
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, borisp@nvidia.com,
john.fastabend@gmail.com, daniel@iogearbox.net,
davejwatson@fb.com, ilyal@mellanox.com, aviadye@mellanox.com,
Jakub Kicinski <kuba@kernel.org>,
Vadim Fedorenko <vfedorenko@novek.ru>,
Seth Forshee <seth.forshee@canonical.com>
Subject: [PATCH net] tls: prevent oversized sendfile() hangs by ignoring MSG_MORE
Date: Fri, 18 Jun 2021 13:34:06 -0700 [thread overview]
Message-ID: <20210618203406.1437414-1-kuba@kernel.org> (raw)
We got multiple reports that multi_chunk_sendfile test
case from tls selftest fails. This was sort of expected,
as the original fix was never applied (see it in the first
Link:). The test in question uses sendfile() with count
larger than the size of the underlying file. This will
make splice set MSG_MORE on all sendpage calls, meaning
TLS will never close and flush the last partial record.
Eric seem to have addressed a similar problem in
commit 35f9c09fe9c7 ("tcp: tcp_sendpages() should call tcp_push() once")
by introducing MSG_SENDPAGE_NOTLAST. Unlike MSG_MORE
MSG_SENDPAGE_NOTLAST is not set on the last call
of a "pipefull" of data (PIPE_DEF_BUFFERS == 16,
so every 16 pages or whenever we run out of data).
Having a break every 16 pages should be fine, TLS
can pack exactly 4 pages into a record, so for
aligned reads there should be no difference,
unaligned may see one extra record per sendpage().
Sticking to TCP semantics seems preferable to modifying
splice, but we can revisit it if real life scenarios
show a regression.
Reported-by: Vadim Fedorenko <vfedorenko@novek.ru>
Reported-by: Seth Forshee <seth.forshee@canonical.com>
Link: https://lore.kernel.org/netdev/1591392508-14592-1-git-send-email-pooja.trivedi@stackpath.com/
Fixes: 3c4d7559159b ("tls: kernel TLS support")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
net/tls/tls_sw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 694de024d0ee..74e5701034aa 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1153,7 +1153,7 @@ static int tls_sw_do_sendpage(struct sock *sk, struct page *page,
int ret = 0;
bool eor;
- eor = !(flags & (MSG_MORE | MSG_SENDPAGE_NOTLAST));
+ eor = !(flags & MSG_SENDPAGE_NOTLAST);
sk_clear_bit(SOCKWQ_ASYNC_NOSPACE, sk);
/* Call the sk_stream functions to manage the sndbuf mem. */
--
2.31.1
next reply other threads:[~2021-06-18 20:34 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-18 20:34 Jakub Kicinski [this message]
2021-06-18 21:04 ` [PATCH net] tls: prevent oversized sendfile() hangs by ignoring MSG_MORE Seth Forshee
2021-06-21 19:50 ` patchwork-bot+netdevbpf
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=20210618203406.1437414-1-kuba@kernel.org \
--to=kuba@kernel.org \
--cc=aviadye@mellanox.com \
--cc=borisp@nvidia.com \
--cc=daniel@iogearbox.net \
--cc=davejwatson@fb.com \
--cc=davem@davemloft.net \
--cc=ilyal@mellanox.com \
--cc=john.fastabend@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=seth.forshee@canonical.com \
--cc=vfedorenko@novek.ru \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).