| Message ID | 20170511000127.4249-1-ycheng@google.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, May 10, 2017 at 8:01 PM, Yuchung Cheng <ycheng@google.com> wrote: > > This patch fixes a bug in splitting an SKB during SACK > processing. Specifically if an skb contains multiple > packets and is only partially sacked in the higher sequences, > tcp_match_sack_to_skb() splits the skb and marks the second fragment > as SACKed. > > The current code further attempts rounding up the first fragment > to MSS boundaries. But it misses a boundary condition when the > rounded-up fragment size (pkt_len) is exactly skb size. Spliting > such an skb is pointless and causses a kernel warning and aborts > the SACK processing. This patch universally checks such over-split > before calling tcp_fragment to prevent these unnecessary warnings. > > Fixes: adb92db857ee ("tcp: Make SACK code to split only at mss boundaries") > Signed-off-by: Yuchung Cheng <ycheng@google.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com> Acked-by: Neal Cardwell <ncardwell@google.com> neal
From: Yuchung Cheng <ycheng@google.com> Date: Wed, 10 May 2017 17:01:27 -0700 > This patch fixes a bug in splitting an SKB during SACK > processing. Specifically if an skb contains multiple > packets and is only partially sacked in the higher sequences, > tcp_match_sack_to_skb() splits the skb and marks the second fragment > as SACKed. > > The current code further attempts rounding up the first fragment > to MSS boundaries. But it misses a boundary condition when the > rounded-up fragment size (pkt_len) is exactly skb size. Spliting > such an skb is pointless and causses a kernel warning and aborts > the SACK processing. This patch universally checks such over-split > before calling tcp_fragment to prevent these unnecessary warnings. > > Fixes: adb92db857ee ("tcp: Make SACK code to split only at mss boundaries") > Signed-off-by: Yuchung Cheng <ycheng@google.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> > Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com> Hehe, a 2.6.29 bug, funny that it lived for so long. Applied and queued up for -stable, thanks!
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 5a3ad09e2786..06e2dbc2b4a2 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -1179,13 +1179,14 @@ static int tcp_match_skb_to_sack(struct sock *sk, struct sk_buff *skb, */ if (pkt_len > mss) { unsigned int new_len = (pkt_len / mss) * mss; - if (!in_sack && new_len < pkt_len) { + if (!in_sack && new_len < pkt_len) new_len += mss; - if (new_len >= skb->len) - return 0; - } pkt_len = new_len; } + + if (pkt_len >= skb->len && !in_sack) + return 0; + err = tcp_fragment(sk, skb, pkt_len, mss, GFP_ATOMIC); if (err < 0) return err;
This patch fixes a bug in splitting an SKB during SACK processing. Specifically if an skb contains multiple packets and is only partially sacked in the higher sequences, tcp_match_sack_to_skb() splits the skb and marks the second fragment as SACKed. The current code further attempts rounding up the first fragment to MSS boundaries. But it misses a boundary condition when the rounded-up fragment size (pkt_len) is exactly skb size. Spliting such an skb is pointless and causses a kernel warning and aborts the SACK processing. This patch universally checks such over-split before calling tcp_fragment to prevent these unnecessary warnings. Fixes: adb92db857ee ("tcp: Make SACK code to split only at mss boundaries") Signed-off-by: Yuchung Cheng <ycheng@google.com> Signed-off-by: Eric Dumazet <edumazet@google.com> Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com> --- net/ipv4/tcp_input.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)