Subject: Re: [PATCH net] tls: prevent oversized sendfile() hangs by ignoring MSG_MORE

From: Seth Forshee <[email-masked]>
Date: Fri, 18 Jun 2021 16:04:07 -0500

On Fri, Jun 18, 2021 at 01:34:06PM -0700, Jakub Kicinski wrote:
> 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 <[email-masked]>
> Reported-by: Seth Forshee <[email-masked]>
> Link: https://lore.kernel.org/netdev/[email-masked]/
> Fixes: 3c4d7559159b ("tls: kernel TLS support")
> Signed-off-by: Jakub Kicinski <[email-masked]>

With this patch the muilt_chunk_sendfile selftest passes. Thanks!

Tested-by: Seth Forshee <[email-masked]>