Date: Mon, 28 Sep 2015 11:08:40 +0200 From: Julien Charbon <jch@freebsd.org> To: John Baldwin <jhb@freebsd.org> Cc: Palle Girgensohn <girgen@freebsd.org>, freebsd-net@freebsd.org Subject: Re: Can tcp_close() be called in INP_TIMEWAIT case Message-ID: <56090398.2070309@freebsd.org> In-Reply-To: <2216936.QIvWsOndvU@ralph.baldwin.cx> References: <26B0FF93-8AE3-4514-BDA1-B966230AAB65@FreeBSD.org> <5602BB7A.9010504@freebsd.org> <5603E8E4.5030406@freebsd.org> <2216936.QIvWsOndvU@ralph.baldwin.cx>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--vwr74Sb8IPl79CrqbqNTJRdLVuT0ekrIh
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: quoted-printable
Hi John,
On 25/09/15 18:42, John Baldwin wrote:
> On Thursday, September 24, 2015 02:13:24 PM Julien Charbon wrote:
>> So the issue is:
>>
>> - tcp_close() is called for some reasons with an inp in INP_TIMEWAIT
>> state and sets the INP_DROPPED flag,
>> - tcp_detach() is called when the last reference on socket is dropped=
>>
>> then now in_pcbfree() can be called twice instead of once:
>>
>> 1. First in tcp_detach():
>>
>> static void
>> tcp_detach(struct socket *so, struct inpcb *inp)
>> {
>> struct tcpcb *tp;
>> tp =3D intotcpcb(inp);
>>
>> if (inp->inp_flags & INP_TIMEWAIT) {
>> if (inp->inp_flags & INP_DROPPED) {
>> in_pcbdetach(inp);
>> in_pcbfree(inp); <--
>> }
>>
>> 2. Second when tcptw expires here:
>>
>> void
>> tcp_twclose(struct tcptw *tw, int reuse)
>> {
>> struct socket *so;
>> struct inpcb *inp;
>>
>> inp =3D tw->tw_inpcb;
>>
>> tcp_tw_2msl_stop(tw, reuse);
>> inp->inp_ppcb =3D NULL;
>> in_pcbdrop(inp);
>>
>> so =3D inp->inp_socket;
>> if (so !=3D NULL) {
>> ...
>> } else {
>> in_pcbfree(inp); <--
>> }
>>
>> This behavior is backed by Palle kernel panic backstraces and coredum=
ps.
>>
>> o Solutions:
>>
>> Long: Forbid to call tcp_close() when inp is in INP_TIMEWAIT state,
>> the TCP stack rule being:
>>
>> - if !INP_TIMEWAIT: Call tcp_close()
>> - if INP_TIMEWAIT: Call tcp_twclose() (or call nothing, the tcptw wil=
l
>> expire/be recycled anyway)
>>
>> Short:
>> if INP_TIMEWAIT & INP_DROPPED:
>> Do not call in_pcbfree(inp) in tcp_detach() unless tcptw is alread=
y
>> discarded.
>>
>> The long solution seems cleaner, backed by tcp_detach() old comments
>> and most of current tcp_close() calls but I won't take that longer pat=
h
>> without -net approval first.
>=20
> I prefer the longer solution if it keeps tcp_detach() simpler by avoidi=
ng
> an extra condition. Please just document it via assertions in tcp_clos=
e()
> (or is this the assertion that fired and triggered the reported panic? =
If so,
> then you obviously don't need to add it. :-P)
Thanks for your answer. And indeed tcp_detach() will be kept simpler
with the longer solution, I will introduce the new assertion in
tcp_close(), something like :
struct tcpcb *
tcp_close(struct tcpcb *tp)
{
...
/*
* tcp_close() should not called on TIME_WAIT connections.
* These connections should be either teardown using
* tcp_twclose(), or left alone waiting for TIME_WAIT timeout
* expiration.
*/
KASSERT((inpb->inp_flags & INP_TIMEWAIT) =3D=3D 0,
("tcp_close cannot be called on TIME_WAIT connections"));
And I will fix all paths that could lead to calling tcp_close() on TCP
TIME_WAIT connections (that is why this solution will be longer). I
found three potential paths so far.
--
Julien
--vwr74Sb8IPl79CrqbqNTJRdLVuT0ekrIh
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"
-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - https://gpgtools.org
iQEcBAEBCgAGBQJWCQOeAAoJEKVlQ5Je6dhxn7cIAJbwLeCyeewBpBaHNaHMuNUl
n1M9mpFG9EpzwP2TFjsSaTGq3UinsVnVAGOweNWGv31oM8izBw3QVB6BKXB5WyPI
aVAin0xF4deEFljGdFYjUgFbu4eZ4Ce8TFXnsnhk7PPdrpNWdUwYA/8XBfyzFGkK
h3mviAT+IZuJSQA32WuNe4B/7z4TEeh3D/o7HKKz7aL1NkvSNd1bWvL2v2trWHun
HoZoiiW6iehUXOHGjtMyTw8s3Q2EDXwCwKnzlKYCW3vNorWR+hNzZlwBxIS3c5nf
Ic8jm1skKN0sELzb9gD90GXFSsbBURWPxFdfMtK65eLumoF+ld9VNbSIsfb4TUs=
=ELxz
-----END PGP SIGNATURE-----
--vwr74Sb8IPl79CrqbqNTJRdLVuT0ekrIh--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?56090398.2070309>