From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David Miller <davem@davemloft.net>,
Yuchung Cheng <ycheng@google.com>,
Oleksandr Natalenko <oleksandr@natalenko.name>,
Roman Gushchin <guro@fb.com>, netdev <netdev@vger.kernel.org>,
Neal Cardwell <ncardwell@google.com>,
Lawrence Brakmo <brakmo@fb.com>
Subject: Re: [PATCH net] tcp: fix tcp_mtu_probe() vs highest_sack
Date: Mon, 30 Oct 2017 23:17:52 -0700 [thread overview]
Message-ID: <20171031061750.al6gjwa7hknefwfy@ast-mbp> (raw)
In-Reply-To: <1509430100.3828.12.camel@edumazet-glaptop3.roam.corp.google.com>
On Mon, Oct 30, 2017 at 11:08:20PM -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Based on SNMP values provided by Roman, Yuchung made the observation
> that some crashes in tcp_sacktag_walk() might be caused by MTU probing.
>
> Looking at tcp_mtu_probe(), I found that when a new skb was placed
> in front of the write queue, we were not updating tcp highest sack.
>
> If one skb is freed because all its content was copied to the new skb
> (for MTU probing), then tp->highest_sack could point to a now freed skb.
>
> Bad things would then happen, including infinite loops.
>
> This patch renames tcp_highest_sack_combine() and uses it
> from tcp_mtu_probe() to fix the bug.
>
> Note that I also removed one test against tp->sacked_out,
> since we want to replace tp->highest_sack regardless of whatever
> condition, since keeping a stale pointer to freed skb is a recipe
> for disaster.
>
> Fixes: a47e5a988a57 ("[TCP]: Convert highest_sack to sk_buff to allow direct access")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
> Reported-by: Roman Gushchin <guro@fb.com>
> Reported-by: Oleksandr Natalenko <oleksandr@natalenko.name>
Thanks!
Acked-by: Alexei Starovoitov <ast@kernel.org>
wow. a bug from 2007.
Any idea why it only started to bite us in 4.11 ?
It's not trivial for us to reproduce it, but we will definitely
test the patch as soon as we can.
Do you have packet drill test or something for easy repro?
next prev parent reply other threads:[~2017-10-31 6:17 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-21 1:46 [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c Roman Gushchin
2017-09-21 17:07 ` Yuchung Cheng
[not found] ` <CAK6E8=cGF+xKiixRVvA=3PVPA7OQta9hVLTgCbKgvYf3e9Eu-A@mail.gmail.com>
2017-09-26 13:10 ` Roman Gushchin
2017-09-27 0:12 ` Yuchung Cheng
2017-09-27 0:18 ` Yuchung Cheng
2017-09-28 8:14 ` Oleksandr Natalenko
2017-09-28 23:36 ` Yuchung Cheng
2017-10-26 2:07 ` Alexei Starovoitov
2017-10-26 5:37 ` Yuchung Cheng
2017-10-27 20:38 ` Eric Dumazet
2017-10-31 6:08 ` [PATCH net] tcp: fix tcp_mtu_probe() vs highest_sack Eric Dumazet
2017-10-31 6:17 ` Alexei Starovoitov [this message]
2017-10-31 6:21 ` Eric Dumazet
2017-10-31 6:30 ` Alexei Starovoitov
2017-11-01 5:50 ` Yuchung Cheng
2017-10-31 13:51 ` Neal Cardwell
2017-11-01 12:20 ` David Miller
2017-11-03 18:22 ` Oleksandr Natalenko
2017-11-03 21:31 ` Eric Dumazet
2017-11-06 22:27 ` [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c Yuchung Cheng
2017-11-10 13:15 ` Oleksandr Natalenko
2017-11-10 13:40 ` Oleksandr Natalenko
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=20171031061750.al6gjwa7hknefwfy@ast-mbp \
--to=alexei.starovoitov@gmail.com \
--cc=brakmo@fb.com \
--cc=davem@davemloft.net \
--cc=eric.dumazet@gmail.com \
--cc=guro@fb.com \
--cc=ncardwell@google.com \
--cc=netdev@vger.kernel.org \
--cc=oleksandr@natalenko.name \
--cc=ycheng@google.com \
/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