Date: Fri, 25 Sep 2015 15:46:35 -0700 From: hiren panchasara <hiren@strugglingcoder.info> To: John Baldwin <jhb@freebsd.org> Cc: Julien Charbon <jch@freebsd.org>, Konstantin Belousov <kostikbel@gmail.com>, George Neville-Neil <gnn@freebsd.org>, Palle Girgensohn <girgen@freebsd.org>, freebsd-net@freebsd.org Subject: Re: Can tcp_close() be called in INP_TIMEWAIT case Message-ID: <20150925224635.GR46700@strugglingcoder.info> 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
[-- Attachment #1 --]
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 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 = 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 = tw->tw_inpcb;
> >
> > tcp_tw_2msl_stop(tw, reuse);
> > inp->inp_ppcb = NULL;
> > in_pcbdrop(inp);
> >
> > so = inp->inp_socket;
> > if (so != NULL) {
> > ...
> > } else {
> > in_pcbfree(inp); <--
> > }
> >
> > This behavior is backed by Palle kernel panic backstraces and coredumps.
> >
> > 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 will
> > expire/be recycled anyway)
> >
> > Short:
> > if INP_TIMEWAIT & INP_DROPPED:
> > Do not call in_pcbfree(inp) in tcp_detach() unless tcptw is already
> > 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 path
> > without -net approval first.
>
> I prefer the longer solution if it keeps tcp_detach() simpler by avoiding
> an extra condition. Please just document it via assertions in tcp_close()
> (or is this the assertion that fired and triggered the reported panic? If so,
> then you obviously don't need to add it. :-P)
I also like the longer solution because it seems cleaner and more
readable/followable.
Julien, nice work on investigation and follow-up. :-)
Cheers,
Hiren
[-- Attachment #2 --]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQF8BAABCgBmBQJWBc7IXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w
ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRBNEUyMEZBMUQ4Nzg4RjNGMTdFNjZGMDI4
QjkyNTBFMTU2M0VERkU1AAoJEIuSUOFWPt/lGroH/jGmVcCO1HOvtoje3poR7pG4
JXtNvztCPATB/cwKL1ufFN8f7Yev9aCyJ6W+grLdl/NYZJxhPelMr1CTNjmDGknW
Iq4aEZhC8NUp64AJJEKBVl5nr4oJDYdhqw4c86Y1OKBY6Ov4sxQ+KQxd2eLE7xsT
EjRvocSTwVecfH6B6OTUzCdYUDGNBD5WoYv+PbW4AhoaoNU9wM1mOQSU6GcBD/Or
KVQHB6IlCrfnu78d/RGbPbbG8TZQ9ITSAl+pQYIL4HlmjCZJ3qcxwSVpjabWC5yY
SU1jBBHpOKmrpv6fdnizQ/7uJOsbjg3yAPY1xZbfZXhSDsQUZBPgKf9iA2fOqsQ=
=MXHc
-----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150925224635.GR46700>