From owner-freebsd-net@freebsd.org Fri Sep 25 16:50:49 2015 Return-Path: Delivered-To: freebsd-net@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id A14D6A089FB for ; Fri, 25 Sep 2015 16:50:49 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) (using TLSv1 with cipher DHE-RSA-CAMELLIA256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 7DD8F11F9; Fri, 25 Sep 2015 16:50:49 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from ralph.baldwin.cx (c-73-231-226-104.hsd1.ca.comcast.net [73.231.226.104]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id BC156B915; Fri, 25 Sep 2015 12:50:47 -0400 (EDT) From: John Baldwin To: Julien Charbon Cc: Palle Girgensohn , George Neville-Neil , Konstantin Belousov , freebsd-net@freebsd.org Subject: Re: Can tcp_close() be called in INP_TIMEWAIT case Date: Fri, 25 Sep 2015 09:42:51 -0700 Message-ID: <2216936.QIvWsOndvU@ralph.baldwin.cx> User-Agent: KMail/4.14.3 (FreeBSD/10.2-PRERELEASE; KDE/4.14.3; amd64; ; ) In-Reply-To: <5603E8E4.5030406@freebsd.org> References: <26B0FF93-8AE3-4514-BDA1-B966230AAB65@FreeBSD.org> <5602BB7A.9010504@freebsd.org> <5603E8E4.5030406@freebsd.org> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Fri, 25 Sep 2015 12:50:47 -0400 (EDT) X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 Sep 2015 16:50:49 -0000 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