From owner-freebsd-net@freebsd.org Fri Sep 25 22:46:43 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 E481CA09309 for ; Fri, 25 Sep 2015 22:46:42 +0000 (UTC) (envelope-from hiren@strugglingcoder.info) Received: from mail.strugglingcoder.info (strugglingcoder.info [65.19.130.35]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mail.strugglingcoder.info", Issuer "mail.strugglingcoder.info" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id C744C1142; Fri, 25 Sep 2015 22:46:42 +0000 (UTC) (envelope-from hiren@strugglingcoder.info) Received: from localhost (unknown [10.1.1.3]) (Authenticated sender: hiren@strugglingcoder.info) by mail.strugglingcoder.info (Postfix) with ESMTPA id 03836D145A; Fri, 25 Sep 2015 15:46:36 -0700 (PDT) Date: Fri, 25 Sep 2015 15:46:35 -0700 From: hiren panchasara To: John Baldwin Cc: Julien Charbon , Konstantin Belousov , George Neville-Neil , Palle Girgensohn , freebsd-net@freebsd.org Subject: Re: Can tcp_close() be called in INP_TIMEWAIT case Message-ID: <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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="uNvczuo8OWfsyO2w" Content-Disposition: inline In-Reply-To: <2216936.QIvWsOndvU@ralph.baldwin.cx> User-Agent: Mutt/1.5.23 (2014-03-12) 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 22:46:43 -0000 --uNvczuo8OWfsyO2w Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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: > >=20 > > - 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 > >=20 > > then now in_pcbfree() can be called twice instead of once: > >=20 > > 1. First in tcp_detach(): > >=20 > > static void > > tcp_detach(struct socket *so, struct inpcb *inp) > > { > > struct tcpcb *tp; > > tp =3D intotcpcb(inp); > >=20 > > if (inp->inp_flags & INP_TIMEWAIT) { > > if (inp->inp_flags & INP_DROPPED) { > > in_pcbdetach(inp); > > in_pcbfree(inp); <-- > > } > >=20 > > 2. Second when tcptw expires here: > >=20 > > void > > tcp_twclose(struct tcptw *tw, int reuse) > > { > > struct socket *so; > > struct inpcb *inp; > >=20 > > inp =3D tw->tw_inpcb; > >=20 > > tcp_tw_2msl_stop(tw, reuse); > > inp->inp_ppcb =3D NULL; > > in_pcbdrop(inp); > >=20 > > so =3D inp->inp_socket; > > if (so !=3D NULL) { > > ... > > } else { > > in_pcbfree(inp); <-- > > } > >=20 > > This behavior is backed by Palle kernel panic backstraces and coredump= s. > >=20 > > o Solutions: > >=20 > > Long: Forbid to call tcp_close() when inp is in INP_TIMEWAIT state, > > the TCP stack rule being: > >=20 > > - if !INP_TIMEWAIT: Call tcp_close() > > - if INP_TIMEWAIT: Call tcp_twclose() (or call nothing, the tcptw will > > expire/be recycled anyway) > >=20 > > Short: > > if INP_TIMEWAIT & INP_DROPPED: > > Do not call in_pcbfree(inp) in tcp_detach() unless tcptw is already > > discarded. > >=20 > > 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. >=20 > 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? I= f 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 --uNvczuo8OWfsyO2w Content-Type: application/pgp-signature -----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----- --uNvczuo8OWfsyO2w--