Re: [PATCH] tcp: be more careful in tcp_fragment()

[Sasha Levin <sashal@kernel.org>]

On Tue, Aug 06, 2019 at 05:25:35PM +0200, Eric Dumazet wrote:
>On Tue, Aug 6, 2019 at 5:09 PM Matthieu Baerts
><matthieu.baerts@tessares.net> wrote:
>>
>> From: Eric Dumazet <edumazet@google.com>
>>
>> commit b617158dc096709d8600c53b6052144d12b89fab upstream.
>>
>> Some applications set tiny SO_SNDBUF values and expect
>> TCP to just work. Recent patches to address CVE-2019-11478
>> broke them in case of losses, since retransmits might
>> be prevented.
>>
>> We should allow these flows to make progress.
>>
>> This patch allows the first and last skb in retransmit queue
>> to be split even if memory limits are hit.
>>
>> It also adds the some room due to the fact that tcp_sendmsg()
>> and tcp_sendpage() might overshoot sk_wmem_queued by about one full
>> TSO skb (64KB size). Note this allowance was already present
>> in stable backports for kernels < 4.15
>>
>> Note for < 4.15 backports :
>>  tcp_rtx_queue_tail() will probably look like :
>>
>> static inline struct sk_buff *tcp_rtx_queue_tail(const struct sock *sk)
>> {
>>         struct sk_buff *skb = tcp_send_head(sk);
>>
>>         return skb ? tcp_write_queue_prev(sk, skb) : tcp_write_queue_tail(sk);
>> }
>>
>> Fixes: f070ef2ac667 ("tcp: tcp_fragment() should apply sane memory limits")
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Reported-by: Andrew Prout <aprout@ll.mit.edu>
>> Tested-by: Andrew Prout <aprout@ll.mit.edu>
>> Tested-by: Jonathan Lemon <jonathan.lemon@gmail.com>
>> Tested-by: Michal Kubecek <mkubecek@suse.cz>
>> Acked-by: Neal Cardwell <ncardwell@google.com>
>> Acked-by: Yuchung Cheng <ycheng@google.com>
>> Acked-by: Christoph Paasch <cpaasch@apple.com>
>> Cc: Jonathan Looney <jtl@netflix.com>
>> Signed-off-by: David S. Miller <davem@davemloft.net>
>> Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
>> ---
>>
>> Notes:
>>     Hello,
>>
>>     Here is the backport for linux-4.14.y branch simply by implementing
>>     functions written by Eric here in the commit message and in this email
>>     thread. It might be valid for older versions, I didn't check.
>>
>>     In my setup with MPTCP, I had the same bug other had where TCP flows
>>     were stalled. The initial fix b6653b3629e5 (tcp: refine memory limit
>>     test in tcp_fragment()) from Eric was helping but the backport in
>>     < 4.15 was not looking safe: 1bc13903773b (tcp: refine memory limit
>>     test in tcp_fragment()).
>>
>>     I then decided to test the new fix and it is working fine in my test
>>     environment, no stalled TCP flows in a few hours.
>>
>>     In this email thread I see that Eric will push a patch for v4.14. I
>>     absolutely do not want to add pressure or steal his work but because I
>>     have the patch here and it is tested, I was thinking it could be a good
>>     idea to share it with others. Feel free to ignore this patch if needed.
>>     But if this patch can reduce a tiny bit Eric's workload, I would be
>>     very glad if it helps :)
>>     Because at the end, it is Eric's work, feel free to change my
>>     "Signed-off-by" by "Tested-By" if it is how it work or nothing if you
>>     prefer to wait for Eric's patch.
>
>This patch is fine, I was simply on vacation last week, and wanted to
>truly take full advantage of them ;)

Queued for 4.14, 4.9, and 4.4. Thanks all!

--
Thanks,
Sasha