From owner-freebsd-net@freebsd.org  Fri Sep 25 22:46:43 2015
Return-Path: <owner-freebsd-net@freebsd.org>
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 <freebsd-net@mailman.ysv.freebsd.org>;
 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 <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>
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 <freebsd-net.freebsd.org>
List-Unsubscribe: <https://lists.freebsd.org/mailman/options/freebsd-net>,
 <mailto:freebsd-net-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-net/>
List-Post: <mailto:freebsd-net@freebsd.org>
List-Help: <mailto:freebsd-net-request@freebsd.org?subject=help>
List-Subscribe: <https://lists.freebsd.org/mailman/listinfo/freebsd-net>,
 <mailto:freebsd-net-request@freebsd.org?subject=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--