Date: Mon, 28 Sep 2015 11:59:10 +0200 From: Julien Charbon <julien.charbon@gmail.com> To: hiren panchasara <hiren@strugglingcoder.info> Cc: freebsd-net@freebsd.org, Palle Girgensohn <girgen@freebsd.org> Subject: Re: Can tcp_close() be called in INP_TIMEWAIT case Message-ID: <56090F6E.3000700@gmail.com> In-Reply-To: <20150925224635.GR46700@strugglingcoder.info> References: <26B0FF93-8AE3-4514-BDA1-B966230AAB65@FreeBSD.org> <5602BB7A.9010504@freebsd.org> <5603E8E4.5030406@freebsd.org> <2216936.QIvWsOndvU@ralph.baldwin.cx> <20150925224635.GR46700@strugglingcoder.info>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--G0VmSHuAHNFtkf8Alj94KjV62nLmgpbs4
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: quoted-printable
Hi Hiren,
On 26/09/15 00:46, hiren panchasara wrote:
> On 09/25/15 at 09:42P, 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 droppe=
d
>>>
>>> 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 coredu=
mps.
>>>
>>> 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 wi=
ll
>>> expire/be recycled anyway)
>>>
>>> Short:
>>> if INP_TIMEWAIT & INP_DROPPED:
>>> Do not call in_pcbfree(inp) in tcp_detach() unless tcptw is alrea=
dy
>>> 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 pa=
th
>>> without -net approval first.
>>
>> I prefer the longer solution if it keeps tcp_detach() simpler by avoid=
ing
>> an extra condition. Please just document it via assertions in tcp_clo=
se()
>> (or is this the assertion that fired and triggered the reported panic?=
If so,
>> then you obviously don't need to add it. :-P)
>=20
> I also like the longer solution because it seems cleaner and more
> readable/followable.
Thanks for your answer. Will do this change and create a review for it.=
> Julien, nice work on investigation and follow-up. :-)
Thanks!
--
Julien
--G0VmSHuAHNFtkf8Alj94KjV62nLmgpbs4
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
iQEcBAEBCgAGBQJWCQ96AAoJEKVlQ5Je6dhxDxIIALop6JagdDm5QhBOvdpX0hXz
pjTV+vCp6V8rNVD2VGVhFpNz8xk3Z9YPvNr63nK8o1LdYNJs6s9+V1CqhaGNqulF
g/lRnjedRSNnV415968PttIbepYLmnBE/4lVSQ8TET7XiT+b6bolUzXXl4AzZvQD
+cTFm4hqu5jKr5tBjI3uqR4OwKFf7rf5LSIQjskRYEs1w6ZI//svk807PTX6MGhR
9kf0eMvwX+bDbIp9X6TIvodbzc5tTLssV3Egs5TbjUKLCkbkqN7KqrHA704ZSQQH
SBqEtTZo0bRE195u9IW++FUgswCfjYT13pBKpnRZrfjEUvzp1Xrsi7jRJpUAwFA=
=Tx4I
-----END PGP SIGNATURE-----
--G0VmSHuAHNFtkf8Alj94KjV62nLmgpbs4--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?56090F6E.3000700>