Date: Fri, 25 Sep 2015 09:42:51 -0700 From: John Baldwin <jhb@freebsd.org> To: Julien Charbon <jch@freebsd.org> Cc: Palle Girgensohn <girgen@freebsd.org>, George Neville-Neil <gnn@freebsd.org>, Konstantin Belousov <kostikbel@gmail.com>, freebsd-net@freebsd.org Subject: Re: Can tcp_close() be called in INP_TIMEWAIT case Message-ID: <2216936.QIvWsOndvU@ralph.baldwin.cx> In-Reply-To: <5603E8E4.5030406@freebsd.org> References: <26B0FF93-8AE3-4514-BDA1-B966230AAB65@FreeBSD.org> <5602BB7A.9010504@freebsd.org> <5603E8E4.5030406@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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)
--
John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2216936.QIvWsOndvU>