Re: [PATCH 4.14] tcp: refine memory limit test in tcp_fragment()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/25/19 3:18 PM, Greg KH wrote:
On Tue, Jun 25, 2019 at 01:29:35PM -0700, Josh Hunt wrote:
On 6/25/19 1:26 PM, Sasha Levin wrote:
On Tue, Jun 25, 2019 at 01:19:37PM -0400, Josh Hunt wrote:
Backport of dad3a9314ac95dedc007bc7dacacb396ea10e376:

You probably meant b6653b3629e5b88202be3c9abc44713973f5c4b4 here.

I wasn't sure if I should reference the upstream commit or stable commit.

The upstream commit please.

Thanks. I'll fix for next version.


dad3a9314 is the version of the commit from linux-4.14.y. There may be a
similar issue with the Fixes tag below since that also references the 4.14
vers of the change.


tcp_fragment() might be called for skbs in the write queue.

Memory limits might have been exceeded because tcp_sendmsg() only
checks limits at full skb (64KB) boundaries.

Therefore, we need to make sure tcp_fragment() wont punish applications
that might have setup very low SO_SNDBUF values.

Backport notes:
Initial version used tcp_queue type which is not present in older
kernels,
so added a new arg to tcp_fragment() to determine whether this is a
retransmit or not.

Fixes: 9daf226ff926 ("tcp: tcp_fragment() should apply sane memory
limits")
Signed-off-by: Josh Hunt <johunt@xxxxxxxxxx>
Reviewed-by: Jason Baron <jbaron@xxxxxxxxxx>
---

Eric/Greg - This applies on top of v4.14.130. I did not see anything come
through for the older (<4.19) stable kernels yet. Without this change
Christoph Paasch's packetrill script (https://lore.kernel.org/netdev/CALMXkpYVRxgeqarp4gnmX7GqYh1sWOAt6UaRFqYBOaaNFfZ5sw@xxxxxxxxxxxxxx/)

will fail on 4.14 stable kernels, but passes with this change.

Eric, it would be great if you could Ack this, it's very different from
your original patch.

Yes, that would be great.

I would prefer if this looks a bit more like the upstream fix, perhaps a
backport of the function that added the "direction" of the packet first,
and then Eric's patch?  As it is, this patch adds a different parameter
to the function than what is in Linus's tree, and I bet will cause
problems at some later point in time.

The commit which introduced the fn arguments is part of a much larger change that created a separate rb-tree for the retransmit queue:

commit 75c119afe14f74b4dd967d75ed9f57ab6c0ef045
Author: Eric Dumazet <edumazet@xxxxxxxxxx>
Date:   Thu Oct 5 22:21:27 2017 -0700

    tcp: implement rb-tree based retransmit queue

I can backport the portion of this change which basically does this:

+enum tcp_queue {
+       TCP_FRAG_IN_WRITE_QUEUE,
+       TCP_FRAG_IN_RTX_QUEUE,
+};
+int tcp_fragment(struct sock *sk, enum tcp_queue tcp_queue,
+                struct sk_buff *skb, u32 len,
+                unsigned int mss_now, gfp_t gfp);

and the corresponding call-sites of tcp_fragment(). If we do that then Eric's fix (b6653b3629e5b88202be3c9abc44713973f5c4b4) should apply cleanly on top of linux-4.14.y. I'm happy to do that if you'd rather go that route. If you want the full rb-tree change into 4.14 then I would defer that to Eric, but would argue that IMHO is probably too invasive of a change for a LTS kernel.

Thanks
Josh



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux