Can tcp_close() be called in INP_TIMEWAIT case

[hiren panchasara]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 603 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-net/attachments/20150925/b2fec59d/attachment.bin>